Diff
Modified: branches/safari-606-branch/LayoutTests/ChangeLog (238284 => 238285)
--- branches/safari-606-branch/LayoutTests/ChangeLog 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/LayoutTests/ChangeLog 2018-11-16 17:28:48 UTC (rev 238285)
@@ -1,3 +1,51 @@
+2018-11-16 Kocsen Chung <kocsen_ch...@apple.com>
+
+ Cherry-pick r238119. rdar://problem/45997459
+
+ 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.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238119 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-13 Alan Coon <alanc...@apple.com>
Cherry-pick r237837. rdar://problem/45996796
Added: branches/safari-606-branch/LayoutTests/fast/inline/marker-list-item-move-should-not-crash-expected.txt (0 => 238285)
--- branches/safari-606-branch/LayoutTests/fast/inline/marker-list-item-move-should-not-crash-expected.txt (rev 0)
+++ branches/safari-606-branch/LayoutTests/fast/inline/marker-list-item-move-should-not-crash-expected.txt 2018-11-16 17:28:48 UTC (rev 238285)
@@ -0,0 +1 @@
+Pass if no crash.
Added: branches/safari-606-branch/LayoutTests/fast/inline/marker-list-item-move-should-not-crash.html (0 => 238285)
--- branches/safari-606-branch/LayoutTests/fast/inline/marker-list-item-move-should-not-crash.html (rev 0)
+++ branches/safari-606-branch/LayoutTests/fast/inline/marker-list-item-move-should-not-crash.html 2018-11-16 17:28:48 UTC (rev 238285)
@@ -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: branches/safari-606-branch/Source/WebCore/ChangeLog (238284 => 238285)
--- branches/safari-606-branch/Source/WebCore/ChangeLog 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/Source/WebCore/ChangeLog 2018-11-16 17:28:48 UTC (rev 238285)
@@ -1,3 +1,66 @@
+2018-11-16 Kocsen Chung <kocsen_ch...@apple.com>
+
+ Cherry-pick r238119. rdar://problem/45997459
+
+ 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.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238119 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-13 Alan Coon <alanc...@apple.com>
Cherry-pick r237837. rdar://problem/45996796
Modified: branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp (238284 => 238285)
--- branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp 2018-11-16 17:28:48 UTC (rev 238285)
@@ -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: branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.h (238284 => 238285)
--- branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.h 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.h 2018-11-16 17:28:48 UTC (rev 238285)
@@ -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: branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp (238284 => 238285)
--- branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp 2018-11-16 17:28:48 UTC (rev 238285)
@@ -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: branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h (238284 => 238285)
--- branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h 2018-11-16 17:28:48 UTC (rev 238285)
@@ -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: branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp (238284 => 238285)
--- branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp 2018-11-16 17:12:44 UTC (rev 238284)
+++ branches/safari-606-branch/Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp 2018-11-16 17:28:48 UTC (rev 238285)
@@ -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));
