Title: [238312] releases/WebKitGTK/webkit-2.22
Revision
238312
Author
mcatanz...@igalia.com
Date
2018-11-16 15:22:06 -0800 (Fri, 16 Nov 2018)

Log Message

Merge r238119 - Do not collapse the soon-to-be-parent anon block when we shuffle around the marker item renderer.
https://bugs.webkit.org/show_bug.cgi?id=191554
<rdar://problem/45825265>

Reviewed by Antti Koivisto.

Source/WebCore:

While moving the marker item renderer to its correct subtree, we accidentally remove the soon-to-be parent anonymous block.
Moving a renderer is a 2 step process:
1. Detach the renderer from its current parent
2. Attach it to its new parent.
During step #1, we check if there is a chance to collapse anonymous blocks. In this case the soon-to-be-parent is a sibling anonymous block which, after detaching the marker sibling
is not needed anymore (except we use it as the new parent).

Test: fast/inline/marker-list-item-move-should-not-crash.html

* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::detach):
* rendering/updating/RenderTreeBuilder.h:
* rendering/updating/RenderTreeBuilderBlock.cpp:
(WebCore::RenderTreeBuilder::Block::detach):
* rendering/updating/RenderTreeBuilderBlock.h:
* rendering/updating/RenderTreeBuilderList.cpp:
(WebCore::RenderTreeBuilder::List::updateItemMarker):

LayoutTests:

* fast/inline/marker-list-item-move-should-not-crash-expected.txt: Added.
* fast/inline/marker-list-item-move-should-not-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog	2018-11-16 23:22:06 UTC (rev 238312)
@@ -1,3 +1,14 @@
+2018-11-12  Zalan Bujtas  <za...@apple.com>
+
+        Do not collapse the soon-to-be-parent anon block when we shuffle around the marker item renderer.
+        https://bugs.webkit.org/show_bug.cgi?id=191554
+        <rdar://problem/45825265>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/inline/marker-list-item-move-should-not-crash-expected.txt: Added.
+        * fast/inline/marker-list-item-move-should-not-crash.html: Added.
+
 2018-11-05  Eric Carlson  <eric.carl...@apple.com>
 
         [MediaStream] An audio track should be muted when capture is interrupted by the OS.

Added: releases/WebKitGTK/webkit-2.22/LayoutTests/fast/inline/marker-list-item-move-should-not-crash-expected.txt (0 => 238312)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/fast/inline/marker-list-item-move-should-not-crash-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/fast/inline/marker-list-item-move-should-not-crash-expected.txt	2018-11-16 23:22:06 UTC (rev 238312)
@@ -0,0 +1 @@
+Pass if no crash.

Added: releases/WebKitGTK/webkit-2.22/LayoutTests/fast/inline/marker-list-item-move-should-not-crash.html (0 => 238312)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/fast/inline/marker-list-item-move-should-not-crash.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/fast/inline/marker-list-item-move-should-not-crash.html	2018-11-16 23:22:06 UTC (rev 238312)
@@ -0,0 +1,13 @@
+<style>
+:matches(::marker) {
+   all: inherit;
+}
+</style>
+
+<li>Pass if no crash.</li>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+document.body.offsetHeight;
+document.linkColor = "foobar";
+</script>

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog	2018-11-16 23:22:06 UTC (rev 238312)
@@ -1,3 +1,29 @@
+2018-11-12  Zalan Bujtas  <za...@apple.com>
+
+        Do not collapse the soon-to-be-parent anon block when we shuffle around the marker item renderer.
+        https://bugs.webkit.org/show_bug.cgi?id=191554
+        <rdar://problem/45825265>
+
+        Reviewed by Antti Koivisto.
+
+        While moving the marker item renderer to its correct subtree, we accidentally remove the soon-to-be parent anonymous block.
+        Moving a renderer is a 2 step process:
+        1. Detach the renderer from its current parent
+        2. Attach it to its new parent.
+        During step #1, we check if there is a chance to collapse anonymous blocks. In this case the soon-to-be-parent is a sibling anonymous block which, after detaching the marker sibling
+        is not needed anymore (except we use it as the new parent).
+
+        Test: fast/inline/marker-list-item-move-should-not-crash.html
+
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::RenderTreeBuilder::detach):
+        * rendering/updating/RenderTreeBuilder.h:
+        * rendering/updating/RenderTreeBuilderBlock.cpp:
+        (WebCore::RenderTreeBuilder::Block::detach):
+        * rendering/updating/RenderTreeBuilderBlock.h:
+        * rendering/updating/RenderTreeBuilderList.cpp:
+        (WebCore::RenderTreeBuilder::List::updateItemMarker):
+
 2018-11-05  Eric Carlson  <eric.carl...@apple.com>
 
         [MediaStream] An audio track should be muted when capture is interrupted by the OS.

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp	2018-11-16 23:22:06 UTC (rev 238312)
@@ -317,7 +317,7 @@
     attach(parent, WTFMove(child), beforeChild);
 }
 
-RenderPtr<RenderObject> RenderTreeBuilder::detach(RenderElement& parent, RenderObject& child)
+RenderPtr<RenderObject> RenderTreeBuilder::detach(RenderElement& parent, RenderObject& child, CanCollapseAnonymousBlock canCollapseAnonymousBlock)
 {
     if (is<RenderRubyAsInline>(parent))
         return rubyBuilder().detach(downcast<RenderRubyAsInline>(parent), child);
@@ -350,10 +350,10 @@
         return svgBuilder().detach(downcast<RenderSVGRoot>(parent), child);
 
     if (is<RenderBlockFlow>(parent))
-        return blockBuilder().detach(downcast<RenderBlockFlow>(parent), child);
+        return blockBuilder().detach(downcast<RenderBlockFlow>(parent), child, canCollapseAnonymousBlock);
 
     if (is<RenderBlock>(parent))
-        return blockBuilder().detach(downcast<RenderBlock>(parent), child);
+        return blockBuilder().detach(downcast<RenderBlock>(parent), child, canCollapseAnonymousBlock);
 
     return detachFromRenderElement(parent, child);
 }

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilder.h (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilder.h	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilder.h	2018-11-16 23:22:06 UTC (rev 238312)
@@ -44,7 +44,8 @@
     void attach(RenderTreePosition&, RenderPtr<RenderObject>);
     void attach(RenderElement& parent, RenderPtr<RenderObject>, RenderObject* beforeChild = nullptr);
 
-    RenderPtr<RenderObject> detach(RenderElement&, RenderObject&) WARN_UNUSED_RETURN;
+    enum class CanCollapseAnonymousBlock { No, Yes };
+    RenderPtr<RenderObject> detach(RenderElement&, RenderObject&, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes) WARN_UNUSED_RETURN;
 
     void destroy(RenderObject& renderer);
 

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp	2018-11-16 23:22:06 UTC (rev 238312)
@@ -271,7 +271,7 @@
     // anonymousBlock is dead here.
 }
 
-RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlock& parent, RenderObject& oldChild)
+RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlock& parent, RenderObject& oldChild, CanCollapseAnonymousBlock canCollapseAnonymousBlock)
 {
     // No need to waste time in merging or removing empty anonymous blocks.
     // We can just bail out if our document is getting destroyed.
@@ -328,27 +328,27 @@
         }
     }
 
-    RenderObject* child = prev ? prev.get() : next.get();
-    if (canMergeAnonymousBlocks && child && !child->previousSibling() && !child->nextSibling() && parent.canDropAnonymousBlockChild()) {
-        // The removal has knocked us down to containing only a single anonymous
-        // box. We can pull the content right back up into our box.
-        dropAnonymousBoxChild(parent, downcast<RenderBlock>(*child));
-    } else if (((prev && prev->isAnonymousBlock()) || (next && next->isAnonymousBlock())) && parent.canDropAnonymousBlockChild()) {
-        // It's possible that the removal has knocked us down to a single anonymous
-        // block with floating siblings.
-        RenderBlock& anonBlock = downcast<RenderBlock>((prev && prev->isAnonymousBlock()) ? *prev : *next);
-        if (canDropAnonymousBlock(anonBlock)) {
-            bool dropAnonymousBlock = true;
-            for (auto& sibling : childrenOfType<RenderObject>(parent)) {
-                if (&sibling == &anonBlock)
-                    continue;
-                if (!sibling.isFloating()) {
-                    dropAnonymousBlock = false;
-                    break;
+    if (canCollapseAnonymousBlock == CanCollapseAnonymousBlock::Yes && parent.canDropAnonymousBlockChild()) {
+        RenderObject* child = prev ? prev.get() : next.get();
+        if (canMergeAnonymousBlocks && child && !child->previousSibling() && !child->nextSibling()) {
+            // The removal has knocked us down to containing only a single anonymous box. We can pull the content right back up into our box.
+            dropAnonymousBoxChild(parent, downcast<RenderBlock>(*child));
+        } else if ((prev && prev->isAnonymousBlock()) || (next && next->isAnonymousBlock())) {
+            // It's possible that the removal has knocked us down to a single anonymous block with floating siblings.
+            RenderBlock& anonBlock = downcast<RenderBlock>((prev && prev->isAnonymousBlock()) ? *prev : *next);
+            if (canDropAnonymousBlock(anonBlock)) {
+                bool dropAnonymousBlock = true;
+                for (auto& sibling : childrenOfType<RenderObject>(parent)) {
+                    if (&sibling == &anonBlock)
+                        continue;
+                    if (!sibling.isFloating()) {
+                        dropAnonymousBlock = false;
+                        break;
+                    }
                 }
+                if (dropAnonymousBlock)
+                    dropAnonymousBoxChild(parent, anonBlock);
             }
-            if (dropAnonymousBlock)
-                dropAnonymousBoxChild(parent, anonBlock);
         }
     }
 
@@ -372,7 +372,7 @@
     child.deleteLines();
 }
 
-RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlockFlow& parent, RenderObject& child)
+RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlockFlow& parent, RenderObject& child, CanCollapseAnonymousBlock canCollapseAnonymousBlock)
 {
     if (!parent.renderTreeBeingDestroyed()) {
         auto* fragmentedFlow = parent.multiColumnFlow();
@@ -379,7 +379,7 @@
         if (fragmentedFlow && fragmentedFlow != &child)
             m_builder.multiColumnBuilder().multiColumnRelativeWillBeRemoved(*fragmentedFlow, child);
     }
-    return detach(static_cast<RenderBlock&>(parent), child);
+    return detach(static_cast<RenderBlock&>(parent), child, canCollapseAnonymousBlock);
 }
 
 }

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h	2018-11-16 23:22:06 UTC (rev 238312)
@@ -37,8 +37,8 @@
     void attach(RenderBlock& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild);
     void attachIgnoringContinuation(RenderBlock& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild);
 
-    RenderPtr<RenderObject> detach(RenderBlock& parent, RenderObject& oldChild) WARN_UNUSED_RETURN;
-    RenderPtr<RenderObject> detach(RenderBlockFlow& parent, RenderObject& child) WARN_UNUSED_RETURN;
+    RenderPtr<RenderObject> detach(RenderBlock& parent, RenderObject& oldChild, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes) WARN_UNUSED_RETURN;
+    RenderPtr<RenderObject> detach(RenderBlockFlow& parent, RenderObject& child, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes) WARN_UNUSED_RETURN;
 
     void dropAnonymousBoxChild(RenderBlock& parent, RenderBlock& child);
     void childBecameNonInline(RenderBlock& parent, RenderElement& child);

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp (238311 => 238312)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp	2018-11-16 23:19:46 UTC (rev 238311)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp	2018-11-16 23:22:06 UTC (rev 238312)
@@ -115,7 +115,7 @@
         return;
 
     if (currentParent)
-        m_builder.attach(*newParent, m_builder.detach(*currentParent, *markerRenderer), firstNonMarkerChild(*newParent));
+        m_builder.attach(*newParent, m_builder.detach(*currentParent, *markerRenderer, RenderTreeBuilder::CanCollapseAnonymousBlock::No), firstNonMarkerChild(*newParent));
     else
         m_builder.attach(*newParent, WTFMove(newMarkerRenderer), firstNonMarkerChild(*newParent));
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to