Changeset 239096 in webkit


Ignore:
Timestamp:
Dec 11, 2018 7:58:41 PM (5 years ago)
Author:
rniwa@webkit.org
Message:

connectedCallback is invoked during the removal of the element inside another element's connectedCallback
https://bugs.webkit.org/show_bug.cgi?id=183586
<rdar://problem/38403504>

Reviewed by Frédéric Wang.

Source/WebCore:

Align WebKit's behavior with Chrome/Firefox with regards to https://github.com/w3c/webcomponents/issues/760

After much discussion, it's unclear that there is a clear path forward to fixing the oddness that
the presence of a custom element reaction changes the timing at which another reaction callback gets invoked.
So matching Chrome/Firefox behaviors in this case seems the path of the least resistance to interoperability.

Namely, this patch makes WebKit not insert a custom element to the appropriate element queue when the element
does not have a matching reaction callback. Put it another way, steps 3-5 in would be done before step 6 in:
https://html.spec.whatwg.org/multipage/custom-elements.html#enqueue-a-custom-element-callback-reaction

  1. Let definition be element's custom element definition.
  2. Let callback be the value of the entry in definition's lifecycle callbacks with key callbackName.
  3. If callback is null, then return
  4. If callbackName is "attributeChangedCallback", then:
    1. Let attributeName be the first element of args.
    2. If definition's observed attributes does not contain attributeName, then return.
  5. Add a new callback reaction to element's custom element reaction queue, with callback function callback and arguments args.
  6. Enqueue an element on the appropriate element queue given element.

Test: fast/custom-elements/enqueue-custom-element-callback-reactions-inside-another-callback.html

  • dom/CustomElementReactionQueue.cpp:

(WebCore::CustomElementReactionQueue::enqueueElementUpgrade):
(WebCore::CustomElementReactionQueue::enqueueConnectedCallbackIfNeeded):
(WebCore::CustomElementReactionQueue::enqueueDisconnectedCallbackIfNeeded):
(WebCore::CustomElementReactionQueue::enqueueAdoptedCallbackIfNeeded):
(WebCore::CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded):
(WebCore::CustomElementReactionQueue::enqueuePostUpgradeReactions):
(WebCore::CustomElementReactionQueue::enqueueElementOnAppropriateElementQueue): Renamed from ensureCurrentQueue.

  • dom/CustomElementReactionQueue.h:

LayoutTests:

Added a W3C style testharness test.

  • fast/custom-elements/enqueue-custom-element-callback-reactions-inside-another-callback-expected.txt: Added.
  • fast/custom-elements/enqueue-custom-element-callback-reactions-inside-another-callback.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r239094 r239096  
     12018-12-10  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        connectedCallback is invoked during the removal of the element inside another element's connectedCallback
     4        https://bugs.webkit.org/show_bug.cgi?id=183586
     5        <rdar://problem/38403504>
     6
     7        Reviewed by Frédéric Wang.
     8
     9        Added a W3C style testharness test.
     10
     11        * fast/custom-elements/enqueue-custom-element-callback-reactions-inside-another-callback-expected.txt: Added.
     12        * fast/custom-elements/enqueue-custom-element-callback-reactions-inside-another-callback.html: Added.
     13
    1142018-12-11  Justin Fan  <justin_fan@apple.com>
    215
  • trunk/Source/WebCore/ChangeLog

    r239094 r239096  
     12018-12-10  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        connectedCallback is invoked during the removal of the element inside another element's connectedCallback
     4        https://bugs.webkit.org/show_bug.cgi?id=183586
     5        <rdar://problem/38403504>
     6
     7        Reviewed by Frédéric Wang.
     8
     9        Align WebKit's behavior with Chrome/Firefox with regards to https://github.com/w3c/webcomponents/issues/760
     10
     11        After much discussion, it's unclear that there is a clear path forward to fixing the oddness that
     12        the presence of a custom element reaction changes the timing at which another reaction callback gets invoked.
     13        So matching Chrome/Firefox behaviors in this case seems the path of the least resistance to interoperability.
     14
     15        Namely, this patch makes WebKit not insert a custom element to the appropriate element queue when the element
     16        does not have a matching reaction callback. Put it another way, steps 3-5 in would be done before step 6 in:
     17        https://html.spec.whatwg.org/multipage/custom-elements.html#enqueue-a-custom-element-callback-reaction
     18            1. Let definition be element's custom element definition.
     19            2. Let callback be the value of the entry in definition's lifecycle callbacks with key callbackName.
     20            3. If callback is null, then return
     21            4. If callbackName is "attributeChangedCallback", then:
     22                1. Let attributeName be the first element of args.
     23                2. If definition's observed attributes does not contain attributeName, then return.
     24            5. Add a new callback reaction to element's custom element reaction queue, with callback function callback
     25               and arguments args.
     26            6. Enqueue an element on the appropriate element queue given element.
     27
     28        Test: fast/custom-elements/enqueue-custom-element-callback-reactions-inside-another-callback.html
     29
     30        * dom/CustomElementReactionQueue.cpp:
     31        (WebCore::CustomElementReactionQueue::enqueueElementUpgrade):
     32        (WebCore::CustomElementReactionQueue::enqueueConnectedCallbackIfNeeded):
     33        (WebCore::CustomElementReactionQueue::enqueueDisconnectedCallbackIfNeeded):
     34        (WebCore::CustomElementReactionQueue::enqueueAdoptedCallbackIfNeeded):
     35        (WebCore::CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded):
     36        (WebCore::CustomElementReactionQueue::enqueuePostUpgradeReactions):
     37        (WebCore::CustomElementReactionQueue::enqueueElementOnAppropriateElementQueue): Renamed from ensureCurrentQueue.
     38        * dom/CustomElementReactionQueue.h:
     39
    1402018-12-11  Justin Fan  <justin_fan@apple.com>
    241
  • trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp

    r236446 r239096  
    121121{
    122122    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
    123     auto& queue = ensureCurrentQueue(element);
     123    ASSERT(element.reactionQueue());
     124    auto& queue = *element.reactionQueue();
    124125    if (alreadyScheduledToUpgrade) {
    125126        ASSERT(queue.m_items.size() == 1);
    126127        ASSERT(queue.m_items[0].type() == CustomElementReactionQueueItem::Type::ElementUpgrade);
    127     } else
     128    } else {
    128129        queue.m_items.append({CustomElementReactionQueueItem::Type::ElementUpgrade});
     130        enqueueElementOnAppropriateElementQueue(element);
     131    }
    129132}
    130133
     
    153156    ASSERT(element.isDefinedCustomElement());
    154157    ASSERT(element.document().refCount() > 0);
    155     auto& queue = ensureCurrentQueue(element);
    156     if (queue.m_interface->hasConnectedCallback())
    157         queue.m_items.append({CustomElementReactionQueueItem::Type::Connected});
     158    ASSERT(element.reactionQueue());
     159    auto& queue = *element.reactionQueue();
     160    if (!queue.m_interface->hasConnectedCallback())
     161        return;
     162    queue.m_items.append({CustomElementReactionQueueItem::Type::Connected});
     163    enqueueElementOnAppropriateElementQueue(element);
    158164}
    159165
     
    164170    if (element.document().refCount() <= 0)
    165171        return; // Don't enqueue disconnectedCallback if the entire document is getting destructed.
    166     auto& queue = ensureCurrentQueue(element);
    167     if (queue.m_interface->hasDisconnectedCallback())
    168         queue.m_items.append({CustomElementReactionQueueItem::Type::Disconnected});
     172    ASSERT(element.reactionQueue());
     173    auto& queue = *element.reactionQueue();
     174    if (!queue.m_interface->hasDisconnectedCallback())
     175        return;
     176    queue.m_items.append({CustomElementReactionQueueItem::Type::Disconnected});
     177    enqueueElementOnAppropriateElementQueue(element);
    169178}
    170179
     
    174183    ASSERT(element.isDefinedCustomElement());
    175184    ASSERT(element.document().refCount() > 0);
    176     auto& queue = ensureCurrentQueue(element);
    177     if (queue.m_interface->hasAdoptedCallback())
    178         queue.m_items.append({oldDocument, newDocument});
     185    ASSERT(element.reactionQueue());
     186    auto& queue = *element.reactionQueue();
     187    if (!queue.m_interface->hasAdoptedCallback())
     188        return;
     189    queue.m_items.append({oldDocument, newDocument});
     190    enqueueElementOnAppropriateElementQueue(element);
    179191}
    180192
     
    184196    ASSERT(element.isDefinedCustomElement());
    185197    ASSERT(element.document().refCount() > 0);
    186     auto& queue = ensureCurrentQueue(element);
    187     if (queue.m_interface->observesAttribute(attributeName.localName()))
    188         queue.m_items.append({attributeName, oldValue, newValue});
     198    ASSERT(element.reactionQueue());
     199    auto& queue = *element.reactionQueue();
     200    if (!queue.m_interface->observesAttribute(attributeName.localName()))
     201        return;
     202    queue.m_items.append({attributeName, oldValue, newValue});
     203    enqueueElementOnAppropriateElementQueue(element);
    189204}
    190205
     
    196211        return;
    197212
    198     auto* queue = element.reactionQueue();
    199     ASSERT(queue);
     213    ASSERT(element.reactionQueue());
     214    auto& queue = *element.reactionQueue();
    200215
    201216    if (element.hasAttributes()) {
    202217        for (auto& attribute : element.attributesIterator()) {
    203             if (queue->m_interface->observesAttribute(attribute.localName()))
    204                 queue->m_items.append({attribute.name(), nullAtom(), attribute.value()});
     218            if (queue.m_interface->observesAttribute(attribute.localName()))
     219                queue.m_items.append({attribute.name(), nullAtom(), attribute.value()});
    205220        }
    206221    }
    207222
    208     if (element.isConnected() && queue->m_interface->hasConnectedCallback())
    209         queue->m_items.append({CustomElementReactionQueueItem::Type::Connected});
     223    if (element.isConnected() && queue.m_interface->hasConnectedCallback())
     224        queue.m_items.append({CustomElementReactionQueueItem::Type::Connected});
    210225}
    211226
     
    274289}
    275290
    276 CustomElementReactionQueue& CustomElementReactionQueue::ensureCurrentQueue(Element& element)
     291// https://html.spec.whatwg.org/multipage/custom-elements.html#enqueue-an-element-on-the-appropriate-element-queue
     292void CustomElementReactionQueue::enqueueElementOnAppropriateElementQueue(Element& element)
    277293{
    278294    ASSERT(element.reactionQueue());
     
    280296        auto& queue = ensureBackupQueue();
    281297        queue.add(element);
    282         return *element.reactionQueue();
     298        return;
    283299    }
    284300
     
    287303        queue = new ElementQueue;
    288304    queue->add(element);
    289     return *element.reactionQueue();
    290305}
    291306
  • trunk/Source/WebCore/dom/CustomElementReactionQueue.h

    r236376 r239096  
    7878
    7979private:
    80     static CustomElementReactionQueue& ensureCurrentQueue(Element&);
     80    static void enqueueElementOnAppropriateElementQueue(Element&);
    8181    static ElementQueue& ensureBackupQueue();
    8282    static ElementQueue& backupElementQueue();
Note: See TracChangeset for help on using the changeset viewer.