Changeset 201858 in webkit

Timestamp:

Jun 9, 2016 2:09:50 AM (8 years ago)

Author:

rniwa@webkit.org

Message:

slotchange event should be fired at the end of microtask
​https://bugs.webkit.org/show_bug.cgi?id=157374
<rdar://problem/26154024>

Reviewed by Antti Koivisto.

Source/WebCore:

Dispatch slotchange event at the end of every microtask after delivering records to mutation observers
as specified in: ​https://dom.spec.whatwg.org/#notify-mutation-observers

Test: fast/shadow-dom/slotchange-event.html

• dom/Document.cpp:

(WebCore::Document::enqueueSlotchangeEvent): Deleted.

• dom/Document.h:
• dom/MutationObserver.cpp:

(WebCore::signalSlotList): Added.
(WebCore::MutationObserverMicrotask::run): mutationObserverCompoundMicrotaskQueuedFlag is now unset in
notifyMutationObservers to better match the concept to "notify mutation observers".
(WebCore::MutationObserver::enqueueSlotChangeEvent): Added.
(WebCore::MutationObserver::notifyMutationObservers): Renamed from deliverAllMutations. Added the code
to dispatch slotchange events as spec'ed, and also added comments for each step.

• dom/MutationObserver.h:
• html/HTMLSlotElement.cpp:

(WebCore::HTMLSlotElement::enqueueSlotChangeEvent): Use MutationObserver::enqueueSlotChangeEvent. Don't
create an event here since that is only needed when dispatching the event, and to keep track of whether
we've already scheduled an event or not. Use a boolean flag instead for the latter.
(WebCore::HTMLSlotElement::dispatchSlotChangeEvent): Added. Creates and dispatches an event.
(WebCore::HTMLSlotElement::dispatchEvent): Deleted.

• html/HTMLSlotElement.h:

(WebCore::HTMLSlotElement::didRemoveFromSignalSlotList): Added.

LayoutTests:

Added a test case to ensure slotchange event is dispatched at the end of a microtask.

• fast/shadow-dom/slotchange-event-expected.txt:
• fast/shadow-dom/slotchange-event.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/shadow-dom/slotchange-event-expected.txt (modified) (1 diff)
• LayoutTests/fast/shadow-dom/slotchange-event.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (1 diff)
• Source/WebCore/dom/Document.h (modified) (1 diff)
• Source/WebCore/dom/MutationObserver.cpp (modified) (7 diffs)
• Source/WebCore/dom/MutationObserver.h (modified) (2 diffs)
• Source/WebCore/html/HTMLSlotElement.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLSlotElement.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-06-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        slotchange event should be fired at the end of microtask
+        https://bugs.webkit.org/show_bug.cgi?id=157374
+        <rdar://problem/26154024>
+
+        Reviewed by Antti Koivisto.
+
+        Added a test case to ensure slotchange event is dispatched at the end of a microtask.
+
+        * fast/shadow-dom/slotchange-event-expected.txt:
+        * fast/shadow-dom/slotchange-event.html:
+
 2016-06-08  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/LayoutTests/fast/shadow-dom/slotchange-event-expected.txt

@@ -30 +30 @@
 PASS slotchange event must fire on a slot element inside an open shadow root  not in a document when nested slots's contents change 
 PASS slotchange event must fire on a slot element inside a closed shadow root  not in a document when nested slots's contents change 
+PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside an open shadow root  in a document when slots's contents change 
+PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside a closed shadow root  in a document when slots's contents change 
+PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside an open shadow root  not in a document when slots's contents change 
+PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside a closed shadow root  not in a document when slots's contents change 
 

trunk/LayoutTests/fast/shadow-dom/slotchange-event.html

@@ -535 +535 @@
 testSlotchangeFiresWhenNestedSlotChange('closed', false);
 
+function testSlotchangeFiresAtEndOfMicroTask(mode, connectedToDocument)
+{
+    var test = async_test('slotchange event must fire at the end of current microtask after mutation observers are invoked inside '
+        + treeName(mode, connectedToDocument) + ' when slots\'s contents change');
+
+    var outerHost;
+    var innerHost;
+    var outerSlot;
+    var innerSlot;
+    var slotchangeEvents = [];
+
+    test.step(function () {
+        outerHost = document.createElement('div');
+        if (connectedToDocument)
+            document.body.appendChild(outerHost);
+
+        var outerShadow = outerHost.attachShadow({'mode': mode});
+        outerShadow.appendChild(document.createElement('span'));
+        outerSlot = document.createElement('slot');
+        outerSlot.addEventListener('slotchange', function (event) {
+            event.stopPropagation();
+            test.step(function () {
+                assert_equals(event.target, outerSlot, 'slotchange event\'s target must be the slot element');
+            });
+            slotchangeEvents.push('outer');
+        });
+
+        innerHost = document.createElement('div');
+        innerHost.appendChild(outerSlot);
+        outerShadow.appendChild(innerHost);
+
+        var innerShadow = innerHost.attachShadow({'mode': mode});
+        innerShadow.appendChild(document.createElement('span'));
+        innerSlot = document.createElement('slot');
+        innerSlot.addEventListener('slotchange', function (event) {
+            event.stopPropagation();
+            test.step(function () {
+                assert_equals(event.target, innerSlot, 'slotchange event\'s target must be the slot element');
+            });
+            slotchangeEvents.push('inner');
+        });
+        innerShadow.appendChild(innerSlot);
+
+        outerHost.appendChild(document.createElement('span'));
+
+        assert_equals(slotchangeEvents.length, 0, 'slotchange event must not be fired synchronously');
+    });
+
+    var element = document.createElement('div');
+
+    new MutationObserver(function () {
+        test.step(function () {
+            assert_equals(slotchangeEvents.length, 0, 'slotchange event must not be fired before mutation records are delivered');
+        });
+        element.setAttribute('title', 'bar');
+        innerHost.appendChild(document.createElement('span'));
+    }).observe(element, {attributes: true, attributeFilter: ['id']});
+
+    new MutationObserver(function () {
+        test.step(function () {
+            assert_array_equals(slotchangeEvents, ['outer', 'inner'], 'slotchange event must be fired during a single compound microtask');
+        });
+    }).observe(element, {attributes: true, attributeFilter: ['title']});
+
+    element.setAttribute('id', 'foo');
+
+    setTimeout(function () {
+        test.step(function () {
+            assert_array_equals(slotchangeEvents, ['outer', 'inner', 'inner'],
+                'a distinct slotchange event must be enqueued for changes made during a mutation observer delivery');
+        });
+        test.done();
+    }, 0);
+}
+
+testSlotchangeFiresAtEndOfMicroTask('open', true);
+testSlotchangeFiresAtEndOfMicroTask('closed', true);
+testSlotchangeFiresAtEndOfMicroTask('open', false);
+testSlotchangeFiresAtEndOfMicroTask('closed', false);
+
 </script>
 </body>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-06-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        slotchange event should be fired at the end of microtask
+        https://bugs.webkit.org/show_bug.cgi?id=157374
+        <rdar://problem/26154024>
+
+        Reviewed by Antti Koivisto.
+
+        Dispatch slotchange event at the end of every microtask after delivering records to mutation observers
+        as specified in: https://dom.spec.whatwg.org/#notify-mutation-observers
+
+        Test: fast/shadow-dom/slotchange-event.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::enqueueSlotchangeEvent): Deleted.
+        * dom/Document.h:
+        * dom/MutationObserver.cpp:
+        (WebCore::signalSlotList): Added.
+        (WebCore::MutationObserverMicrotask::run): mutationObserverCompoundMicrotaskQueuedFlag is now unset in
+        notifyMutationObservers to better match the concept to "notify mutation observers".
+        (WebCore::MutationObserver::enqueueSlotChangeEvent): Added.
+        (WebCore::MutationObserver::notifyMutationObservers): Renamed from deliverAllMutations. Added the code
+        to dispatch slotchange events as spec'ed, and also added comments for each step.
+        * dom/MutationObserver.h:
+        * html/HTMLSlotElement.cpp:
+        (WebCore::HTMLSlotElement::enqueueSlotChangeEvent): Use MutationObserver::enqueueSlotChangeEvent. Don't
+        create an event here since that is only needed when dispatching the event, and to keep track of whether
+        we've already scheduled an event or not. Use a boolean flag instead for the latter. 
+        (WebCore::HTMLSlotElement::dispatchSlotChangeEvent): Added. Creates and dispatches an event.
+        (WebCore::HTMLSlotElement::dispatchEvent): Deleted.
+        * html/HTMLSlotElement.h:
+        (WebCore::HTMLSlotElement::didRemoveFromSignalSlotList): Added.
+
 2016-06-08  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -4251 +4251 @@
 }
 
-void Document::enqueueSlotchangeEvent(Ref<Event>&& event)
-{
-    m_eventQueue.enqueueEvent(WTFMove(event));
-}
-
 RefPtr<Event> Document::createEvent(const String& type, ExceptionCode& ec)
 {

trunk/Source/WebCore/dom/Document.h

@@ -1090 +1090 @@
     void enqueueDocumentEvent(Ref<Event>&&);
     void enqueueOverflowEvent(Ref<Event>&&);
-    void enqueueSlotchangeEvent(Ref<Event>&&);
     void enqueuePageshowEvent(PageshowEventPersistence);
     void enqueueHashchangeEvent(const String& oldURL, const String& newURL);

trunk/Source/WebCore/dom/MutationObserver.cpp

@@ -36 +36 @@
 #include "Document.h"
 #include "ExceptionCode.h"
+#include "HTMLSlotElement.h"
 #include "Microtasks.h"
 #include "MutationCallback.h"
@@ -147 +148 @@
 }
 
+// https://dom.spec.whatwg.org/#signal-slot-list
+static Vector<RefPtr<HTMLSlotElement>>& signalSlotList()
+{
+    static NeverDestroyed<Vector<RefPtr<HTMLSlotElement>>> list;
+    return list;
+}
+
 static bool mutationObserverCompoundMicrotaskQueuedFlag;
 
@@ -154 +162 @@
     Result run() final
     {
-        mutationObserverCompoundMicrotaskQueuedFlag = false;
-        MutationObserver::deliverAllMutations();
+        MutationObserver::notifyMutationObservers();
         return Result::Done;
     }
@@ -173 +180 @@
     m_records.append(WTFMove(mutation));
     activeMutationObservers().add(this);
+
+    queueMutationObserverCompoundMicrotask();
+}
+
+void MutationObserver::enqueueSlotChangeEvent(HTMLSlotElement& slot)
+{
+    ASSERT(isMainThread());
+    ASSERT(!signalSlotList().contains(&slot));
+    signalSlotList().append(&slot);
 
     queueMutationObserverCompoundMicrotask();
@@ -221 +237 @@
 }
 
-void MutationObserver::deliverAllMutations()
-{
+void MutationObserver::notifyMutationObservers()
+{
+    // https://dom.spec.whatwg.org/#notify-mutation-observers
+    // 1. Unset mutation observer compound microtask queued flag.
+    mutationObserverCompoundMicrotaskQueuedFlag = false;
+
     ASSERT(isMainThread());
     static bool deliveryInProgress = false;
@@ -241 +261 @@
     }
 
-    while (!activeMutationObservers().isEmpty()) {
-        Vector<RefPtr<MutationObserver>> observers;
-        copyToVector(activeMutationObservers(), observers);
+    while (!activeMutationObservers().isEmpty() || !signalSlotList().isEmpty()) {
+        // 2. Let notify list be a copy of unit of related similar-origin browsing contexts' list of MutationObserver objects.
+        Vector<RefPtr<MutationObserver>> notifyList;
+        copyToVector(activeMutationObservers(), notifyList);
         activeMutationObservers().clear();
-        std::sort(observers.begin(), observers.end(), [](auto& lhs, auto& rhs) {
+        std::sort(notifyList.begin(), notifyList.end(), [](auto& lhs, auto& rhs) {
             return lhs->m_priority < rhs->m_priority;
         });
 
-        for (auto& observer : observers) {
+        // 3. Let signalList be a copy of unit of related similar-origin browsing contexts' signal slot list.
+        // 4. Empty unit of related similar-origin browsing contexts' signal slot list.
+        Vector<RefPtr<HTMLSlotElement>> slotList;
+        if (!signalSlotList().isEmpty()) {
+            slotList.swap(signalSlotList());
+            for (auto& slot : slotList)
+                slot->didRemoveFromSignalSlotList();
+        }
+
+        // 5. For each MutationObserver object mo in notify list, execute a compound microtask subtask
+        for (auto& observer : notifyList) {
             if (observer->canDeliver())
                 observer->deliver();
@@ -255 +286 @@
                 suspendedMutationObservers().add(observer);
         }
+
+        // 6. For each slot slot in signalList, in order, fire an event named slotchange, with its bubbles attribute set to true, at slot.
+        for (auto& slot : slotList)
+            slot->dispatchSlotChangeEvent();
     }
 

trunk/Source/WebCore/dom/MutationObserver.h

@@ -40 +40 @@
 namespace WebCore {
 
+class HTMLSlotElement;
 class MutationCallback;
 class MutationObserverRegistration;
@@ -97 +98 @@
     HashSet<Node*> observedNodes() const;
 
+    static void enqueueSlotChangeEvent(HTMLSlotElement&);
+
 private:
     explicit MutationObserver(Ref<MutationCallback>&&);
     void deliver();
 
-    static void deliverAllMutations();
+    static void notifyMutationObservers();
     static bool validateOptions(MutationObserverOptions);
 

trunk/Source/WebCore/html/HTMLSlotElement.cpp

@@ -32 +32 @@
 #include "EventNames.h"
 #include "HTMLNames.h"
+#include "MutationObserver.h"
 #include "ShadowRoot.h"
 
@@ -127 +128 @@
 void HTMLSlotElement::enqueueSlotChangeEvent()
 {
-    if (m_enqueuedSlotChangeEvent)
+    // https://dom.spec.whatwg.org/#signal-a-slot-change
+    if (m_inSignalSlotList)
         return;
+    m_inSignalSlotList = true;
+    MutationObserver::enqueueSlotChangeEvent(*this);
+}
+
+void HTMLSlotElement::dispatchSlotChangeEvent()
+{
+    m_inSignalSlotList = false;
 
     bool bubbles = false;
     bool cancelable = false;
-    auto event = Event::create(eventNames().slotchangeEvent, bubbles, cancelable);
+    Ref<Event> event = Event::create(eventNames().slotchangeEvent, bubbles, cancelable);
     event->setTarget(this);
-    m_enqueuedSlotChangeEvent = event.ptr();
-    document().enqueueSlotchangeEvent(WTFMove(event));
-}
-
-bool HTMLSlotElement::dispatchEvent(Event& event)
-{
-    if (&event == m_enqueuedSlotChangeEvent)
-        m_enqueuedSlotChangeEvent = nullptr;
-    return HTMLElement::dispatchEvent(event);
+    dispatchEvent(event);
 }
 

trunk/Source/WebCore/html/HTMLSlotElement.h

@@ -43 +43 @@
 
     void enqueueSlotChangeEvent();
+    void didRemoveFromSignalSlotList() { m_inSignalSlotList = false; }
+
+    void dispatchSlotChangeEvent();
 
 private:
@@ -50 +53 @@
     void removedFrom(ContainerNode&) final;
     void attributeChanged(const QualifiedName&, const AtomicString& oldValue, const AtomicString& newValue, AttributeModificationReason) final;
-    bool dispatchEvent(Event&) final;
 
-    Event* m_enqueuedSlotChangeEvent { nullptr };
+    bool m_inSignalSlotList { false };
 };