Changeset 190323 in webkit

Timestamp:

Sep 29, 2015 1:22:18 PM (9 years ago)

Author:

rniwa@webkit.org

Message:

Update style/layout when a slot is added or removed
​https://bugs.webkit.org/show_bug.cgi?id=149593

Reviewed by Antti Koivisto.

Source/WebCore:

Fixed the bug by forcing the render tree reconstruction on the shadow host when a slot is inserted or removed.
We should optimize these reconstructions by only triggering them on the affected slot elements in the future.

Also fixed a bug that we were not invalidating the slot assignments when a default slot is introduced dynamically
after the slot assignment algorithm had run.

Test (existing): fast/shadow-dom/shadow-layout-after-slot-changes.html

• dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::addSlotElementByName): Passes in ShadowRoot.
(WebCore::ShadowRoot::removeSlotElementByName): Ditto.

• dom/SlotAssignment.cpp:

(WebCore::SlotAssignment::addSlotElementByName): Call setNeedsStyleRecalc.
(WebCore::SlotAssignment::removeSlotElementByName): Call setNeedsStyleRecalc if the host is still alive since this
function can be called while the host is being destructed in which case shadowRoot.host() would be nullptr.

• dom/SlotAssignment.h:

LayoutTests:

Removed failing test expectations from fast/shadow-dom/shadow-layout-after-slot-changes.html

Also added an explicit test case for when a default slot is introduced dynamically after
calling getDistributedNodes() once, thereby forcing the slot assignments.

• fast/shadow-dom/HTMLSlotElement-interface-expected.txt:
• fast/shadow-dom/HTMLSlotElement-interface.html:
• fast/shadow-dom/shadow-layout-after-slot-changes.html:
• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/shadow-dom/HTMLSlotElement-interface-expected.txt (modified) (1 diff)
• LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/ShadowRoot.cpp (modified) (2 diffs)
• Source/WebCore/dom/SlotAssignment.cpp (modified) (4 diffs)
• Source/WebCore/dom/SlotAssignment.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-09-29  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Update style/layout when a slot is added or removed
+        https://bugs.webkit.org/show_bug.cgi?id=149593
+
+        Reviewed by Antti Koivisto.
+
+        Removed failing test expectations from fast/shadow-dom/shadow-layout-after-slot-changes.html
+
+        Also added an explicit test case for when a default slot is introduced dynamically after
+        calling getDistributedNodes() once, thereby forcing the slot assignments.
+
+        * fast/shadow-dom/HTMLSlotElement-interface-expected.txt:
+        * fast/shadow-dom/HTMLSlotElement-interface.html:
+        * fast/shadow-dom/shadow-layout-after-slot-changes.html:
+        * platform/mac/TestExpectations:
+
 2015-09-29  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface-expected.txt

@@ -4 +4 @@
 PASS getDistributedNodes method on HTMLSlotElement must return the list of distributed nodes 
 PASS getDistributedNodes must update when slot and name attributes are modified 
+PASS getDistributedNodes must update when a default slot is introduced dynamically by a slot rename 
 PASS getDistributedNodes must update when slot elements are inserted or removed 
 

trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html

@@ -93 +93 @@
 test(function () {
     var shadowHost = document.createElement('div');
+    var child = document.createElement('span');
+    shadowHost.appendChild(child);
+
+    var shadowRoot = shadowHost.attachShadow({mode: 'open'});
+    var slotElement = document.createElement('slot');
+    slotElement.name = 'foo';
+    shadowRoot.appendChild(slotElement);
+
+    assert_array_equals(slotElement.getDistributedNodes(), [], 'getDistributedNodes must be empty when there are no matching elements for the slot name');
+
+    slotElement.name = null;
+    assert_array_equals(slotElement.getDistributedNodes(), [child], 'getDistributedNodes must be empty when there are no matching elements for the slot name');
+
+}, 'getDistributedNodes must update when a default slot is introduced dynamically by a slot rename');
+
+test(function () {
+    var shadowHost = document.createElement('div');
     var p = document.createElement('p');
     var text = document.createTextNode('');

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1331 +1331 @@
 webkit.org/b/149441 fast/shadow-dom/css-scoping-shadow-slotted-rule.html [ ImageOnlyFailure ]
 webkit.org/b/149441 fast/shadow-dom/css-scoping-shadow-slot-display-override.html [ ImageOnlyFailure ]
-webkit.org/b/149593 fast/shadow-dom/shadow-layout-after-slot-changes.html [ ImageOnlyFailure ]
 
 webkit.org/b/149510 accessibility/mac/aria-expanded-notifications.html [ Pass Failure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-09-29  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Update style/layout when a slot is added or removed
+        https://bugs.webkit.org/show_bug.cgi?id=149593
+
+        Reviewed by Antti Koivisto.
+
+        Fixed the bug by forcing the render tree reconstruction on the shadow host when a slot is inserted or removed.
+        We should optimize these reconstructions by only triggering them on the affected slot elements in the future.
+
+        Also fixed a bug that we were not invalidating the slot assignments when a default slot is introduced dynamically
+        after the slot assignment algorithm had run.
+
+        Test (existing): fast/shadow-dom/shadow-layout-after-slot-changes.html
+
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::addSlotElementByName): Passes in ShadowRoot.
+        (WebCore::ShadowRoot::removeSlotElementByName): Ditto.
+        * dom/SlotAssignment.cpp:
+        (WebCore::SlotAssignment::addSlotElementByName): Call setNeedsStyleRecalc.
+        (WebCore::SlotAssignment::removeSlotElementByName): Call setNeedsStyleRecalc if the host is still alive since this
+        function can be called while the host is being destructed in which case shadowRoot.host() would be nullptr.
+        * dom/SlotAssignment.h:
+
 2015-09-29  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -1 +1 @@
 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -190 +191 @@
         m_slotAssignments = std::make_unique<SlotAssignment>();
 
-    return m_slotAssignments->addSlotElementByName(name, slot);
+    return m_slotAssignments->addSlotElementByName(name, slot, *this);
 }
 
 void ShadowRoot::removeSlotElementByName(const AtomicString& name, HTMLSlotElement& slot)
 {
-    return m_slotAssignments->removeSlotElementByName(name, slot);
+    return m_slotAssignments->removeSlotElementByName(name, slot, *this);
 }
 

trunk/Source/WebCore/dom/SlotAssignment.cpp

@@ -52 +52 @@
 }
 
-void SlotAssignment::addSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement)
+void SlotAssignment::addSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
 #ifndef NDEBUG
@@ -59 +59 @@
 #endif
 
-    auto addResult = m_slots.add(treatNullAsEmpty(name), std::unique_ptr<SlotInfo>());
+    // FIXME: We should be able to do a targeted reconstruction.
+    shadowRoot.host()->setNeedsStyleRecalc(ReconstructRenderTree);
+
+    const AtomicString& key = treatNullAsEmpty(name);
+    auto addResult = m_slots.add(key, std::unique_ptr<SlotInfo>());
     if (addResult.isNewEntry) {
         addResult.iterator->value = std::make_unique<SlotInfo>(slotElement);
+        if (key == emptyAtom) // Because assignSlots doesn't collect nodes assgined to the default slot as an optimzation.
+            m_slotAssignmentsIsValid = false;
         return;
     }
@@ -78 +84 @@
 }
 
-void SlotAssignment::removeSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement)
+void SlotAssignment::removeSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
 #ifndef NDEBUG
@@ -84 +90 @@
     m_slotElementsForConsistencyCheck.remove(&slotElement);
 #endif
+
+    if (auto* host = shadowRoot.host()) // FIXME: We should be able to do a targeted reconstruction.
+        host->setNeedsStyleRecalc(ReconstructRenderTree);
 
     auto it = m_slots.find(treatNullAsEmpty(name));

trunk/Source/WebCore/dom/SlotAssignment.h

@@ -48 +48 @@
     HTMLSlotElement* findAssignedSlot(const Node&, ShadowRoot&);
 
-    void addSlotElementByName(const AtomicString&, HTMLSlotElement&);
-    void removeSlotElementByName(const AtomicString&, HTMLSlotElement&);
+    void addSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&);
+    void removeSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&);
 
     const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);