invalidateSlotAssignments should trigger style recalc
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Sep 2015 08:17:09 +0000 (08:17 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Sep 2015 08:17:09 +0000 (08:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149447

Reviewed by Antti Koivisto.

Source/WebCore:

Invalidate the render tree of a shadow host when a new child is inserted, an existing child is removed,
or slot attribute of a child is modified.

No new tests. Covered by existing tests added in r190101.

* dom/Element.cpp:
(WebCore::Element::childrenChanged): Call invalidateSlotAssignments or invalidateDefaultSlotAssignments
depending on the types of children being inserted or removed since text nodes can only be assigned into
a default slot.
* dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::invalidateSlotAssignments):
(WebCore::ShadowRoot::invalidateDefaultSlotAssignments): Added.
* dom/ShadowRoot.h:
* dom/SlotAssignment.cpp:
(WebCore::SlotAssignment::invalidate): Reconstruct the render tree for the whole host. We can optimize
in the future by only invalidating active slot elements instead.
(WebCore::SlotAssignment::invalidateDefaultSlot): Added.
* dom/SlotAssignment.h:

LayoutTests:

Removed failing expectations from newly passing tests.

Also added test cases for inserting and removing text nodes, and modified the style recalc tests
to force layout between each DOM change to test case separately.

* fast/shadow-dom/shadow-layout-after-host-child-changes.html:
* fast/shadow-dom/shadow-layout-after-inserting-or-removing-host-child.html:
* fast/shadow-dom/shadow-layout-after-slot-changes.html:
* platform/mac/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/shadow-layout-after-host-child-changes.html
LayoutTests/fast/shadow-dom/shadow-layout-after-inserting-or-removing-host-child.html
LayoutTests/fast/shadow-dom/shadow-layout-after-slot-changes.html
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/ShadowRoot.h
Source/WebCore/dom/SlotAssignment.cpp
Source/WebCore/dom/SlotAssignment.h

index a8702e763c3e76d0be7639c284da07ca9488731b..ea55b4f4ccc5123dbe9a656cc256d28bdada97dd 100644 (file)
@@ -1,3 +1,20 @@
+2015-09-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        invalidateSlotAssignments should trigger style recalc
+        https://bugs.webkit.org/show_bug.cgi?id=149447
+
+        Reviewed by Antti Koivisto.
+
+        Removed failing expectations from newly passing tests.
+
+        Also added test cases for inserting and removing text nodes, and modified the style recalc tests
+        to force layout between each DOM change to test case separately.
+
+        * fast/shadow-dom/shadow-layout-after-host-child-changes.html:
+        * fast/shadow-dom/shadow-layout-after-inserting-or-removing-host-child.html:
+        * fast/shadow-dom/shadow-layout-after-slot-changes.html:
+        * platform/mac/TestExpectations:
+
 2015-09-21  Chris Dumez  <cdumez@apple.com>
 
         time element should use HTMLTimeElement interface
index a48b7eb8f66d1e4e7505f56f6061caebf75bbce3..239137546de1f4cd8adcd98a5f5683616cd9aec2 100644 (file)
@@ -23,6 +23,13 @@ my-host > div {
 </style>
 <script>
 
+function forceLayout() {
+    if (window.internals)
+        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+    else
+        document.querySelector('p').getBoundingClientRect();
+}
+
 try {
     var host1 = document.getElementById('host1');
     host1.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>';
@@ -36,14 +43,20 @@ try {
     var host4 = document.getElementById('host4');
     host4.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>';
 
-    if (window.internals)
-        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
-    else
-        document.querySelector('p').getBoundingClientRect();
+    forceLayout();
 
     host1.removeChild(host1.firstChild);
+
+    forceLayout();
+
     host2.firstChild.slot = 'bar';
+
+    forceLayout();
+
     host3.firstChild.slot = null;
+
+    forceLayout();
+
     var greenBox = document.createElement('div');
     greenBox.style.backgroundColor = 'green';
     host4.insertBefore(greenBox, host4.firstChild);
index 3709519536aa816c535c3ca47e9cb2a0d7e5a892..fe1fda8cb04057aac8aa8a333bece53130a151bb 100644 (file)
@@ -2,39 +2,62 @@
 <html>
 <body>
 <p>Test passes if you see a single 100px by 100px green box below.</p> 
-<green-host style="background: green;"><span>FAIL</span></green-host>
-<red-host style="background: red;"></red-host>
+<my-host id="host1" style="background: green;"><span>FAIL</span></my-host>
+<my-host id="host2" style="background: red;"></my-host>
+<my-host id="host3" style="background: green;">FAIL</my-host>
+<my-host id="host4" style="background: green;">FAIL</my-host>
 <style>
 
-green-host, red-host {
+my-host {
     display: block;
     width: 100px;
-    height: 50px;
+    height: 25px;
     overflow: hidden;
+    font-size: 30px;
 }
 
 </style>
 <script>
 
-try {
-    var greenHost = document.querySelector('green-host');
-    greenHost.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>';
-
-    var redHost = document.querySelector('red-host');
-    redHost.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>FAIL';
-
+function forceLayout() {
     if (window.internals)
         internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
     else
-        document.querySelector('span').getBoundingClientRect();
+        document.querySelector('p').getBoundingClientRect();
+}
+
+try {
+    var host1 = document.getElementById('host1');
+    host1.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>';
+
+    var host2 = document.getElementById('host2');
+    host2.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>FAIL';
+
+    var host3 = document.getElementById('host3');
+    host3.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>';
 
-    greenHost.removeChild(greenHost.firstChild);
+    var host4 = document.getElementById('host4');
+    host4.attachShadow({mode: 'open'}).innerHTML = '<slot></slot>';
+
+    forceLayout();
+
+    host1.removeChild(host1.firstChild);
+
+    forceLayout();
 
     var greenChild = document.createElement('div');
     greenChild.style.width = '100%';
     greenChild.style.height = '100%';
     greenChild.style.backgroundColor = 'green';
-    redHost.appendChild(greenChild);
+    host2.appendChild(greenChild);
+
+    forceLayout();
+
+    host3.removeChild(host3.firstChild);
+
+    forceLayout();
+
+    host4.insertBefore(document.createTextNode('________'), host4.firstChild);
 
 } catch (exception) {
     document.body.appendChild(document.createTextNode(exception));
index 01a6568f06c21d3ef62030b3560693ac1d6b2804..c1d969e866aa596bad71ccfc01f8bef8c718877c 100644 (file)
@@ -23,6 +23,13 @@ my-host > div {
 </style>
 <script>
 
+function forceLayout() {
+    if (window.internals)
+        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+    else
+        document.querySelector('p').getBoundingClientRect();
+}
+
 try {
     var shadow1 = document.getElementById('host1').attachShadow({mode: 'open'});
     shadow1.innerHTML = '<slot></slot>';
@@ -36,14 +43,17 @@ try {
     var shadow4 = document.getElementById('host4').attachShadow({mode: 'open'});
     shadow4.innerHTML = 'FAIL';
 
-    if (window.internals)
-        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
-    else
-        document.querySelector('p').getBoundingClientRect();
+    forceLayout();
 
     shadow1.removeChild(shadow1.firstChild);
+    forceLayout();
+
     shadow2.firstChild.name = 'bar';
+    forceLayout();
+
     shadow3.firstChild.name = null;
+    forceLayout();
+
     shadow4.insertBefore(document.createElement('slot'), shadow4.firstChild);
 
 } catch (exception) {
index a80b7d36ae0bb508724308367b44769894d21002..97e06a7bbffcf483b29f4575e9ece442168b2fbb 100644 (file)
@@ -1308,6 +1308,4 @@ webkit.org/b/149328 fast/shadow-dom/css-scoping-shadow-host-rule.html [ ImageOnl
 webkit.org/b/149328 fast/shadow-dom/css-scoping-shadow-host-functional-rule.html [ ImageOnlyFailure ]
 webkit.org/b/149328 fast/shadow-dom/css-scoping-shadow-slotted-rule.html [ ImageOnlyFailure ]
 webkit.org/b/149328 fast/shadow-dom/css-scoping-shadow-slot-display-override.html [ ImageOnlyFailure ]
-webkit.org/b/149328 fast/shadow-dom/shadow-layout-after-host-child-changes.html [ ImageOnlyFailure ]
-webkit.org/b/149328 fast/shadow-dom/shadow-layout-after-inserting-or-removing-host-child.html [ ImageOnlyFailure ]
 webkit.org/b/149328 fast/shadow-dom/shadow-layout-after-slot-changes.html [ ImageOnlyFailure ]
index f7373c8ad7470a1342fe7c73d0f22a6c539a734c..7b09fd33e7e257ac84981fc119704b03a1106010 100644 (file)
@@ -1,3 +1,29 @@
+2015-09-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        invalidateSlotAssignments should trigger style recalc
+        https://bugs.webkit.org/show_bug.cgi?id=149447
+
+        Reviewed by Antti Koivisto.
+
+        Invalidate the render tree of a shadow host when a new child is inserted, an existing child is removed,
+        or slot attribute of a child is modified.
+
+        No new tests. Covered by existing tests added in r190101.
+
+        * dom/Element.cpp:
+        (WebCore::Element::childrenChanged): Call invalidateSlotAssignments or invalidateDefaultSlotAssignments
+        depending on the types of children being inserted or removed since text nodes can only be assigned into
+        a default slot.
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::invalidateSlotAssignments):
+        (WebCore::ShadowRoot::invalidateDefaultSlotAssignments): Added.
+        * dom/ShadowRoot.h:
+        * dom/SlotAssignment.cpp:
+        (WebCore::SlotAssignment::invalidate): Reconstruct the render tree for the whole host. We can optimize
+        in the future by only invalidating active slot elements instead.
+        (WebCore::SlotAssignment::invalidateDefaultSlot): Added.
+        * dom/SlotAssignment.h:
+
 2015-09-21  Chris Dumez  <cdumez@apple.com>
 
         time element should use HTMLTimeElement interface
index bd3074d1e8374b0b777b6e7043b479bb56846e54..e3f21c3b1a399b3766f0492a88fb2fcbf42fe379 100644 (file)
@@ -1864,7 +1864,20 @@ void Element::childrenChanged(const ChildChange& change)
         if (auto* distributor = shadowRoot->distributor())
             distributor->invalidateDistribution(this);
 #if ENABLE(SHADOW_DOM)
-        shadowRoot->invalidateSlotAssignments();
+        switch (change.type) {
+        case ElementInserted:
+        case ElementRemoved:
+        case AllChildrenRemoved:
+            shadowRoot->invalidateSlotAssignments();
+            break;
+        case TextInserted:
+        case TextRemoved:
+        case TextChanged:
+            shadowRoot->invalidateDefaultSlotAssignments();
+            break;
+        case NonContentsChildChanged:
+            break;
+        }
 #endif
     }
 }
index 04cef5ca1d14837b27af7f7429d67d2fcb70f386..f4bfd6a264c9a3ef9e368e2f575eb86aeeb1a88a 100644 (file)
@@ -166,7 +166,13 @@ void ShadowRoot::removeSlotElementByName(const AtomicString& name, HTMLSlotEleme
 void ShadowRoot::invalidateSlotAssignments()
 {
     if (m_slotAssignments)
-        m_slotAssignments->invalidate();
+        m_slotAssignments->invalidate(*this);
+}
+
+void ShadowRoot::invalidateDefaultSlotAssignments()
+{
+    if (m_slotAssignments)
+        m_slotAssignments->invalidateDefaultSlot(*this);
 }
 
 const Vector<Node*>* ShadowRoot::assignedNodesForSlot(const HTMLSlotElement& slot)
index b5a2da908d2ce7a0d9e1738fc7d4e8ee082fa76b..6b5118530d7d131e747318649c50c0871643a135 100644 (file)
@@ -83,6 +83,7 @@ public:
     void removeSlotElementByName(const AtomicString&, HTMLSlotElement&);
 
     void invalidateSlotAssignments();
+    void invalidateDefaultSlotAssignments();
 
     const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&);
 #endif
index 7a3fd35294028eb5eba1cf591d9ce905579d7fc3..d636e25aa749818650bf4f37c5e47b717ca224ee 100644 (file)
@@ -122,6 +122,20 @@ const Vector<Node*>* SlotAssignment::assignedNodesForSlot(const HTMLSlotElement&
     return &slotInfo.assignedNodes;
 }
 
+void SlotAssignment::invalidate(ShadowRoot& shadowRoot)
+{
+    // FIXME: We should be able to do a targeted reconstruction.
+    shadowRoot.host()->setNeedsStyleRecalc(ReconstructRenderTree);
+    m_slotAssignmentsIsValid = false;
+}
+
+void SlotAssignment::invalidateDefaultSlot(ShadowRoot& shadowRoot)
+{
+    auto it = m_slots.find(emptyAtom);
+    if (it != m_slots.end() && it->value->elementCount)
+        invalidate(shadowRoot); // FIXME: We should be able to reconstruct only under the default slot.
+}
+
 HTMLSlotElement* SlotAssignment::findFirstSlotElement(SlotInfo& slotInfo, ShadowRoot& shadowRoot)
 {
     if (slotInfo.shouldResolveSlotElement())
index 048b93b47edee23eb867707e84563605919018a5..588d26d54053771858228cdbe65811ffd03ef4d7 100644 (file)
@@ -52,7 +52,8 @@ public:
 
     const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);
 
-    void invalidate() { m_slotAssignmentsIsValid = false; }
+    void invalidate(ShadowRoot&);
+    void invalidateDefaultSlot(ShadowRoot&);
 
 private:
     struct SlotInfo {