- Revision
- 190323
- Author
- rn...@webkit.org
- Date
- 2015-09-29 13:22:18 -0700 (Tue, 29 Sep 2015)
Log 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:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (190322 => 190323)
--- trunk/LayoutTests/ChangeLog 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/LayoutTests/ChangeLog 2015-09-29 20:22:18 UTC (rev 190323)
@@ -1,3 +1,20 @@
+2015-09-29 Ryosuke Niwa <rn...@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 <ryanhad...@apple.com>
Update test expectations to mark quicklook/pages.html as crashing
Modified: trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface-expected.txt (190322 => 190323)
--- trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface-expected.txt 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface-expected.txt 2015-09-29 20:22:18 UTC (rev 190323)
@@ -3,5 +3,6 @@
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
Modified: trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html (190322 => 190323)
--- trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html 2015-09-29 20:22:18 UTC (rev 190323)
@@ -92,6 +92,23 @@
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');
Modified: trunk/LayoutTests/platform/mac/TestExpectations (190322 => 190323)
--- trunk/LayoutTests/platform/mac/TestExpectations 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2015-09-29 20:22:18 UTC (rev 190323)
@@ -1330,7 +1330,6 @@
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 ]
Modified: trunk/Source/WebCore/ChangeLog (190322 => 190323)
--- trunk/Source/WebCore/ChangeLog 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/Source/WebCore/ChangeLog 2015-09-29 20:22:18 UTC (rev 190323)
@@ -1,3 +1,27 @@
+2015-09-29 Ryosuke Niwa <rn...@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.fab...@crf.canon.fr>
Improve binding of JSBuiltinConstructor classes
Modified: trunk/Source/WebCore/dom/ShadowRoot.cpp (190322 => 190323)
--- trunk/Source/WebCore/dom/ShadowRoot.cpp 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/Source/WebCore/dom/ShadowRoot.cpp 2015-09-29 20:22:18 UTC (rev 190323)
@@ -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 @@
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()
Modified: trunk/Source/WebCore/dom/SlotAssignment.cpp (190322 => 190323)
--- trunk/Source/WebCore/dom/SlotAssignment.cpp 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/Source/WebCore/dom/SlotAssignment.cpp 2015-09-29 20:22:18 UTC (rev 190323)
@@ -51,16 +51,22 @@
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 @@
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());
Modified: trunk/Source/WebCore/dom/SlotAssignment.h (190322 => 190323)
--- trunk/Source/WebCore/dom/SlotAssignment.h 2015-09-29 19:20:36 UTC (rev 190322)
+++ trunk/Source/WebCore/dom/SlotAssignment.h 2015-09-29 20:22:18 UTC (rev 190323)
@@ -47,8 +47,8 @@
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&);
