Update style/layout when a slot is added or removed
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Sep 2015 20:22:18 +0000 (20:22 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Sep 2015 20:22:18 +0000 (20:22 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190323 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/HTMLSlotElement-interface-expected.txt
LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/SlotAssignment.cpp
Source/WebCore/dom/SlotAssignment.h

index d6d653f..0c3e62b 100644 (file)
@@ -1,3 +1,20 @@
+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>
 
         Update test expectations to mark quicklook/pages.html as crashing
index c223415..572a79d 100644 (file)
@@ -3,5 +3,6 @@ PASS HTMLSlotElement must be defined on window
 PASS "name" attribute on HTMLSlotElement must reflect "name" attribute 
 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 
 
index 56a53b1..699f87a 100644 (file)
@@ -92,6 +92,23 @@ test(function () {
 
 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('');
     var b = document.createElement('b');
index ad71f6e..2ad5d51 100644 (file)
@@ -1330,7 +1330,6 @@ webkit.org/b/149440 fast/shadow-dom/css-scoping-shadow-host-rule.html [ ImageOnl
 webkit.org/b/149440 fast/shadow-dom/css-scoping-shadow-host-functional-rule.html [ ImageOnlyFailure ]
 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 ]
 
index 4a9025d..73a9bea 100644 (file)
@@ -1,3 +1,27 @@
+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>
 
         Improve binding of JSBuiltinConstructor classes
index 71f83b9..fc23978 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * 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
  * modification, are permitted provided that the following conditions are
@@ -189,12 +190,12 @@ void ShadowRoot::addSlotElementByName(const AtomicString& name, HTMLSlotElement&
     if (!m_slotAssignments)
         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);
 }
 
 void ShadowRoot::invalidateSlotAssignments()
index d636e25..764e79b 100644 (file)
@@ -51,16 +51,22 @@ HTMLSlotElement* SlotAssignment::findAssignedSlot(const Node& node, ShadowRoot&
     return findFirstSlotElement(*it->value, shadowRoot);
 }
 
-void SlotAssignment::addSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement)
+void SlotAssignment::addSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
 #ifndef NDEBUG
     ASSERT(!m_slotElementsForConsistencyCheck.contains(&slotElement));
     m_slotElementsForConsistencyCheck.add(&slotElement);
 #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;
     }
 
@@ -77,13 +83,16 @@ void SlotAssignment::addSlotElementByName(const AtomicString& name, HTMLSlotElem
     slotInfo.elementCount++;
 }
 
-void SlotAssignment::removeSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement)
+void SlotAssignment::removeSlotElementByName(const AtomicString& name, HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
 #ifndef NDEBUG
     ASSERT(m_slotElementsForConsistencyCheck.contains(&slotElement));
     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));
     RELEASE_ASSERT(it != m_slots.end());
 
index 588d26d..1ac1f26 100644 (file)
@@ -47,8 +47,8 @@ public:
 
     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&);