Attachment 348477 Details for Bug 189144 – WIP

[patch] WIP

wip189144.patch (text/plain), 22.74 KB, created by Ryosuke Niwa on 2018-08-29 20:47:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-29 20:47:45 PDT

Size: 22.74 KB

patch

obsolete

>Index: Source/WebCore/dom/ContainerNode.cpp
>===================================================================
>--- Source/WebCore/dom/ContainerNode.cpp	(revision 235483)
>+++ Source/WebCore/dom/ContainerNode.cpp	(working copy)
>@@ -59,6 +59,7 @@
> #include "SVGUseElement.h"
> #include "ScriptDisallowedScope.h"
> #include "SelectorQuery.h"
>+#include "SlotAssignment.h"
> #include "TemplateContentDocumentFragment.h"
> #include <algorithm>
> #include <wtf/IsoMallocInlines.h>
>@@ -104,6 +105,9 @@
>     WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
>     ScriptDisallowedScope::InMainThread scriptDisallowedScope;
> 
>+    if (UNLIKELY(isShadowRoot() || isInShadowTree()))
>+        containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval();
>+
>     document().nodeChildrenWillBeRemoved(*this);
> 
>     while (RefPtr<Node> child = m_firstChild) {
>@@ -150,6 +154,9 @@
>         WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
>         ScriptDisallowedScope::InMainThread scriptDisallowedScope;
> 
>+        if (UNLIKELY(isShadowRoot() || isInShadowTree()))
>+            containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval();
>+
>         document().nodeWillBeRemoved(childToRemove);
> 
>         ASSERT_WITH_SECURITY_IMPLICATION(childToRemove.parentNode() == this);
>@@ -181,6 +188,10 @@
>     NodeVector postInsertionNotificationTargets;
>     {
>         ScriptDisallowedScope::InMainThread scriptDisallowedScope;
>+
>+        if (UNLIKELY(containerNode.isShadowRoot() || containerNode.isInShadowTree()))
>+            containerNode.containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval();
>+
>         doNodeInsertion();
>         ChildListMutationScope(containerNode).childAdded(child);
>         postInsertionNotificationTargets = notifyChildNodeInserted(containerNode, child);
>Index: Source/WebCore/dom/ShadowRoot.cpp
>===================================================================
>--- Source/WebCore/dom/ShadowRoot.cpp	(revision 235483)
>+++ Source/WebCore/dom/ShadowRoot.cpp	(working copy)
>@@ -82,6 +82,9 @@
>     // to access it Document reference after that.
>     willBeDeletedFrom(document());
> 
>+    ASSERT(!m_hasBegunDeletingDetachedChildren);
>+    m_hasBegunDeletingDetachedChildren = true;
>+
>     // We must remove all of our children first before the TreeScope destructor
>     // runs so we don't go through Node::setTreeScopeRecursively for each child with a
>     // destructed tree scope in each descendant.
>Index: Source/WebCore/dom/ShadowRoot.h
>===================================================================
>--- Source/WebCore/dom/ShadowRoot.h	(revision 235483)
>+++ Source/WebCore/dom/ShadowRoot.h	(working copy)
>@@ -58,6 +58,10 @@
>     bool resetStyleInheritance() const { return m_resetStyleInheritance; }
>     void setResetStyleInheritance(bool);
> 
>+    bool hasBegunDeletingDetachedChildren() const { return m_hasBegunDeletingDetachedChildren; }
>+
>+    ShadowRootMode mode() const { return m_type; }
>+
>     Element* host() const { return m_host; }
>     void setHost(Element* host) { m_host = host; }
> 
>@@ -66,8 +70,6 @@
> 
>     Element* activeElement() const;
> 
>-    ShadowRootMode mode() const { return m_type; }
>-
>     void removeAllEventListeners() override;
> 
>     HTMLSlotElement* findAssignedSlot(const Node&);
>@@ -75,6 +77,7 @@
>     void addSlotElementByName(const AtomicString&, HTMLSlotElement&);
>     void removeSlotElementByName(const AtomicString&, HTMLSlotElement&);
>     void slotFallbackDidChange(HTMLSlotElement&);
>+    void resolveSlotsBeforeNodeInsertionOrRemoval();
> 
>     void didRemoveAllChildrenOfShadowHost();
>     void didChangeDefaultSlot();
>@@ -103,6 +106,7 @@
>     void removedFromAncestor(RemovalType, ContainerNode& insertionPoint) override;
> 
>     bool m_resetStyleInheritance { false };
>+    bool m_hasBegunDeletingDetachedChildren { false };
>     ShadowRootMode m_type { ShadowRootMode::UserAgent };
> 
>     Element* m_host { nullptr };
>Index: Source/WebCore/dom/SlotAssignment.cpp
>===================================================================
>--- Source/WebCore/dom/SlotAssignment.cpp	(revision 235483)
>+++ Source/WebCore/dom/SlotAssignment.cpp	(working copy)
>@@ -85,13 +85,37 @@
>     });
> 
>     auto& slot = *addResult.iterator->value;
>-    if (!slot.hasSlotElements())
>+    if (!slot.hasSlotElements()) {
>         slot.element = makeWeakPtr(slotElement);
>-    else {
>-        slot.element = nullptr;
>-#ifndef NDEBUG
>-        m_needsToResolveSlotElements = true;
>-#endif
>+        if (shadowRoot.mode() != ShadowRootMode::UserAgent) {
>+            if (!m_slotAssignmentsIsValid)
>+                assignSlots(shadowRoot);
>+            if (!slot.assignedNodes.isEmpty())
>+                slotElement.enqueueSlotChangeEvent();
>+        }
>+    } else {
>+        ASSERT(slot.element); // resolveSlotsBeforeNodeInsertionOrRemoval must have been called.
>+        bool newSlotIsFirstSlot = false;
>+        if (shadowRoot.mode() != ShadowRootMode::UserAgent) {
>+            auto& previouslyFirstSlot = *slot.element;
>+            if (!m_slotAssignmentsIsValid)
>+                assignSlots(shadowRoot);
>+            if (!slot.assignedNodes.isEmpty()) {
>+                HTMLSlotElement* slotBeforePrevioslyFirstSlot = Traversal<HTMLSlotElement>::previous(previouslyFirstSlot);
>+                if (slotBeforePrevioslyFirstSlot == &slotElement) {
>+                    slotElement.enqueueSlotChangeEvent();
>+                    slot.element->enqueueSlotChangeEvent();
>+                    newSlotIsFirstSlot = slot.elementCount == 2;
>+                }
>+            }
>+        }
>+        if (newSlotIsFirstSlot)
>+            slot.element = makeWeakPtr(slotElement);
>+        else {
>+            slot.element = nullptr;
>+            m_needsToResolveSlotElements = true;
>+        }
>+
>     }
>     slot.elementCount++;
> }
>@@ -109,14 +133,29 @@
>     auto* slot = m_slots.get(slotNameFromAttributeValue(name));
>     RELEASE_ASSERT(slot && slot->hasSlotElements());
> 
>+    bool removedFirstSlot = slot->element == &slotElement;
>     slot->elementCount--;
>-    if (slot->element == &slotElement) {
>+    if (removedFirstSlot) {
>         slot->element = nullptr;
>-#ifndef NDEBUG
>         m_needsToResolveSlotElements = true;
>-#endif
>     }
>     ASSERT(slot->element || m_needsToResolveSlotElements);
>+
>+    if (shadowRoot.mode() == ShadowRootMode::UserAgent || !removedFirstSlot || shadowRoot.hasBegunDeletingDetachedChildren())
>+        return;
>+
>+    if (!m_slotAssignmentsIsValid)
>+        assignSlots(shadowRoot);
>+
>+    if (slot->assignedNodes.isEmpty())
>+        return;
>+
>+    slotElement.enqueueSlotChangeEvent();
>+    if (slot->elementCount > 1) {
>+        resolveAllSlotElements(shadowRoot);
>+        ASSERT(slot->element);
>+        slot->element->enqueueSlotChangeEvent();
>+    }
> }
> 
> void SlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
>@@ -129,6 +168,13 @@
>         slotElement.enqueueSlotChangeEvent();
> }
> 
>+void SlotAssignment::resolveSlotsBeforeNodeInsertionOrRemoval(ShadowRoot& shadowRoot)
>+{
>+    ASSERT(shadowRoot.mode() != ShadowRootMode::UserAgent);
>+    if (m_needsToResolveSlotElements)
>+        resolveAllSlotElements(shadowRoot);
>+}
>+
> void SlotAssignment::didChangeSlot(const AtomicString& slotAttrValue, ShadowRoot& shadowRoot)
> {
>     auto& slotName = slotNameFromAttributeValue(slotAttrValue);
>@@ -196,10 +242,8 @@
> 
> void SlotAssignment::resolveAllSlotElements(ShadowRoot& shadowRoot)
> {
>-#ifndef NDEBUG
>     ASSERT(m_needsToResolveSlotElements);
>     m_needsToResolveSlotElements = false;
>-#endif
> 
>     // FIXME: It's inefficient to reset all values. We should be able to void this in common case.
>     for (auto& entry : m_slots)
>Index: Source/WebCore/dom/SlotAssignment.h
>===================================================================
>--- Source/WebCore/dom/SlotAssignment.h	(revision 235483)
>+++ Source/WebCore/dom/SlotAssignment.h	(working copy)
>@@ -53,6 +53,7 @@
>     void addSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&);
>     void removeSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&);
>     void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&);
>+    void resolveSlotsBeforeNodeInsertionOrRemoval(ShadowRoot&);
> 
>     void didChangeSlot(const AtomicString&, ShadowRoot&);
>     void enqueueSlotChangeEvent(const AtomicString&, ShadowRoot&);
>@@ -88,12 +89,18 @@
> 
> #ifndef NDEBUG
>     HashSet<HTMLSlotElement*> m_slotElementsForConsistencyCheck;
>-    bool m_needsToResolveSlotElements { false };
> #endif
> 
>+    bool m_needsToResolveSlotElements { false };
>     bool m_slotAssignmentsIsValid { false };
> };
> 
>+inline void ShadowRoot::resolveSlotsBeforeNodeInsertionOrRemoval()
>+{
>+    if (mode() != ShadowRootMode::UserAgent && m_slotAssignment)
>+        m_slotAssignment->resolveSlotsBeforeNodeInsertionOrRemoval(*this);
>+}
>+
> inline void ShadowRoot::didRemoveAllChildrenOfShadowHost()
> {
>     if (m_slotAssignment) // FIXME: This is incorrect when there were no elements or text nodes removed.
>Index: LayoutTests/fast/shadow-dom/slotchange-event-bubbling.html
>===================================================================
>--- LayoutTests/fast/shadow-dom/slotchange-event-bubbling.html	(revision 235483)
>+++ LayoutTests/fast/shadow-dom/slotchange-event-bubbling.html	(working copy)
>@@ -75,7 +75,7 @@
> test_slotchange_event_bubbles('open', true);
> 
> function test_single_slotchange_event_for_nested_slots(outerMode, innerMode, connected) {
>-    promise_test(() => {
>+    promise_test(async () => {
>         const outerHost = document.createElement('outer-host');
>         if (connected)
>             document.body.appendChild(outerHost);
>@@ -91,6 +91,8 @@
>         const innerSlotParent = innerShadow.querySelector('div');
>         const innerSlot = innerShadow.querySelector('slot');
> 
>+        await Promise.resolve();
>+
>         const observer = create_slotchange_observer();
>         observer.observe(outerSlot);
>         observer.observe(innerHost);
>@@ -109,23 +111,23 @@
>         outerHost.textContent = ' ';
> 
>         assert_array_equals(observer.takeLog(), [], 'slotchange event must not be fired synchronously');
>-        return Promise.resolve().then(() => {
>-            const log = observer.takeLog();
>+        await Promise.resolve();
> 
>-            const events = new Set(log.map((entry) => entry.event));
>-            assert_equals(events.size, 1, 'Mutating the assigned content of a slot must fire exactly one slotchange event');
>+        const log = observer.takeLog();
> 
>-            assert_slotchange_log(log[0], outerSlot, outerSlot, 'slotchange event must be dispatched at the slot element first');
>-            assert_slotchange_log(log[1], innerSlot, outerSlot, 'slotchange event must bubble up from a slot element to its assigned slot');
>-            assert_slotchange_log(log[2], innerSlotParent, outerSlot, 'slotchange event must bubble up to the parent node of a slot');
>-            assert_slotchange_log(log[3], innerShadow, outerSlot, 'slotchange event must bubble up to the shadow root');
>-            assert_slotchange_log(log[4], innerHost, outerSlot,
>-                'slotchange event must bubble up to the shadow host if the host is a descendent of the tree in which the event was fired');
>-            assert_slotchange_log(log[5], outerHostParent, outerSlot,
>-                'slotchange event must bubble up to the parent of an inner shadow host');
>-            assert_slotchange_log(log[6], outerShadow, outerSlot, 'slotchange event must bubble up to the shadow root');
>-            assert_equals(log.length, 7, 'slotchange must not bubble beyond the shadow root in which the event was fired');
>-        });
>+        const events = new Set(log.map((entry) => entry.event));
>+        assert_equals(events.size, 1, 'Mutating the assigned content of a slot must fire exactly one slotchange event');
>+
>+        assert_slotchange_log(log[0], outerSlot, outerSlot, 'slotchange event must be dispatched at the slot element first');
>+        assert_slotchange_log(log[1], innerSlot, outerSlot, 'slotchange event must bubble up from a slot element to its assigned slot');
>+        assert_slotchange_log(log[2], innerSlotParent, outerSlot, 'slotchange event must bubble up to the parent node of a slot');
>+        assert_slotchange_log(log[3], innerShadow, outerSlot, 'slotchange event must bubble up to the shadow root');
>+        assert_slotchange_log(log[4], innerHost, outerSlot,
>+            'slotchange event must bubble up to the shadow host if the host is a descendent of the tree in which the event was fired');
>+        assert_slotchange_log(log[5], outerHostParent, outerSlot,
>+            'slotchange event must bubble up to the parent of an inner shadow host');
>+        assert_slotchange_log(log[6], outerShadow, outerSlot, 'slotchange event must bubble up to the shadow root');
>+        assert_equals(log.length, 7, 'slotchange must not bubble beyond the shadow root in which the event was fired');
>     }, `A single slotchange event must bubble from a ${connected ? 'connected' : 'disconnected'} ${innerMode}-mode shadow tree to`
>         + `a slot in its parent ${outerMode}-mode shadow tree`);
> }
>Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-customelements-expected.txt
>===================================================================
>--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-customelements-expected.txt	(revision 235483)
>+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-customelements-expected.txt	(working copy)
>@@ -1,3 +1,3 @@
> 
>-FAIL slotchange must fire on initialization of custom elements with slotted children assert_true: expected true got false
>+PASS slotchange must fire on initialization of custom elements with slotted children 
> 
>Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-event-expected.txt
>===================================================================
>--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-event-expected.txt	(revision 235483)
>+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-event-expected.txt	(working copy)
>@@ -1,10 +1,4 @@
>-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0
>-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0
>-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0
>-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0
> 
>-Harness Error (FAIL), message = Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0
>-
> PASS slotchange event must fire on a default slot element inside an open shadow root  in a document 
> PASS slotchange event must fire on a default slot element inside a closed shadow root  in a document 
> PASS slotchange event must fire on a default slot element inside an open shadow root  not in a document 
>@@ -17,10 +11,10 @@
> PASS slotchange event must not fire on a slot element inside a closed shadow root  in a document when another slot's assigned nodes change 
> PASS slotchange event must not fire on a slot element inside an open shadow root  not in a document when another slot's assigned nodes change 
> PASS slotchange event must not fire on a slot element inside a closed shadow root  not in a document when another slot's assigned nodes change 
>-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root  in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0
>-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root  in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0
>-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root  not in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0
>-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root  not in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0
>+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root  in a document 
>+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root  in a document 
>+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root  not in a document 
>+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root  not in a document 
> PASS slotchange event must fire on a slot element inside an open shadow root  in a document even if the slot was removed immediately after the assigned nodes were mutated 
> PASS slotchange event must fire on a slot element inside a closed shadow root  in a document even if the slot was removed immediately after the assigned nodes were mutated 
> PASS slotchange event must fire on a slot element inside an open shadow root  not in a document even if the slot was removed immediately after the assigned nodes were mutated 
>@@ -29,10 +23,10 @@
> PASS slotchange event must fire on a slot element inside a closed shadow root  in a document when innerHTML modifies the children of the shadow host 
> PASS slotchange event must fire on a slot element inside an open shadow root  not in a document when innerHTML modifies the children of the shadow host 
> PASS slotchange event must fire on a slot element inside a closed shadow root  not in a document when innerHTML modifies the children of the shadow host 
>-FAIL slotchange event must fire on a slot element inside an open shadow root  in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot>
>-FAIL slotchange event must fire on a slot element inside a closed shadow root  in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot>
>-FAIL slotchange event must fire on a slot element inside an open shadow root  not in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot>
>-FAIL slotchange event must fire on a slot element inside a closed shadow root  not in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot>
>+PASS slotchange event must fire on a slot element inside an open shadow root  in a document when nested slots's contents change 
>+PASS slotchange event must fire on a slot element inside a closed shadow root  in a document when nested slots's contents change 
>+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 
>Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt
>===================================================================
>--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt	(revision 235483)
>+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt	(working copy)
>@@ -1,19 +1,17 @@
> 
>-Harness Error (TIMEOUT), message = null
>-
> PASS slotchange event: Append a child to a host. 
> PASS slotchange event: Remove a child from a host. 
> PASS slotchange event: Remove a child before adding an event listener. 
> PASS slotchange event: Change slot= attribute to make it un-assigned. 
>-TIMEOUT slotchange event: Change slot's name= attribute so that none is assigned. Test timed out
>+PASS slotchange event: Change slot's name= attribute so that none is assigned. 
> PASS slotchange event: Change slot= attribute to make it assigned. 
>-TIMEOUT slotchange event: Change slot's name= attribute so that a node is assigned to the slot. Test timed out
>+PASS slotchange event: Change slot's name= attribute so that a node is assigned to the slot. 
> PASS slotchange event: Add a fallback content. 
> PASS slotchange event: Remove a fallback content. 
> PASS slotchange event: Add a fallback content to nested slots. 
> PASS slotchange event: Remove a fallback content from nested slots. 
>-TIMEOUT slotchange event: Insert a slot before an existing slot. Test timed out
>-TIMEOUT slotchange event: Remove a preceding slot. Test timed out
>+PASS slotchange event: Insert a slot before an existing slot. 
>+PASS slotchange event: Remove a preceding slot. 
> PASS slotchange event: A slot is assigned to another slot. 
> PASS slotchange event: Even if distributed nodes do not change, slotchange should be fired if assigned nodes are changed. 
>

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189144: 348477 | 348486 | 348490 | 348583 | 348594 | 348697 | 348699 | 348709 | 348710 | 348711 | 348839