- Revision
- 261675
- Author
- [email protected]
- Date
- 2020-05-13 21:55:18 -0700 (Wed, 13 May 2020)
Log Message
Do not clear selection/repaint when the renderer gets moved during tree normalization.
https://bugs.webkit.org/show_bug.cgi?id=211865
<rdar://problem/62849044>
Reviewed by Antti Koivisto.
Source/WebCore:
While detachFromRenderElement should really be about "this renderer is being detached from its parent", some code in here assumes
the renderer is not only going to be detached but also going to be destroyed. Ideally such code should live in RenderObject::willBeDestroyed(),
but no tree walk is possible in there anymore.
The reason for the crash is that when we split the inline for continuation, we construct new anonymous containers and move subtrees over
to these new renderers. However at this point these new parent containers are not yet attached to the tree
so any tree-walking cleanup code will fail (in detachFromRenderElement).
Test: fast/repaint/do-no-repaint-on-internal-move.html
* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::detachFromRenderElement):
* rendering/updating/RenderTreeBuilder.h:
* rendering/updating/RenderTreeBuilderInline.cpp:
(WebCore::RenderTreeBuilder::Inline::splitInlines):
LayoutTests:
* fast/repaint/do-no-repaint-on-internal-move-expected.txt: Added.
* fast/repaint/do-no-repaint-on-internal-move.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (261674 => 261675)
--- trunk/LayoutTests/ChangeLog 2020-05-14 04:00:00 UTC (rev 261674)
+++ trunk/LayoutTests/ChangeLog 2020-05-14 04:55:18 UTC (rev 261675)
@@ -1,3 +1,14 @@
+2020-05-13 Zalan Bujtas <[email protected]>
+
+ Do not clear selection/repaint when the renderer gets moved during tree normalization.
+ https://bugs.webkit.org/show_bug.cgi?id=211865
+ <rdar://problem/62849044>
+
+ Reviewed by Antti Koivisto.
+
+ * fast/repaint/do-no-repaint-on-internal-move-expected.txt: Added.
+ * fast/repaint/do-no-repaint-on-internal-move.html: Added.
+
2020-05-13 Lauro Moura <[email protected]>
[GTK][WPE] Move shared imported tests expectation files to glib
Added: trunk/LayoutTests/fast/repaint/do-no-repaint-on-internal-move-expected.txt (0 => 261675)
--- trunk/LayoutTests/fast/repaint/do-no-repaint-on-internal-move-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/repaint/do-no-repaint-on-internal-move-expected.txt 2020-05-14 04:55:18 UTC (rev 261675)
@@ -0,0 +1,3 @@
+Pass
+if no crash.
+
Added: trunk/LayoutTests/fast/repaint/do-no-repaint-on-internal-move.html (0 => 261675)
--- trunk/LayoutTests/fast/repaint/do-no-repaint-on-internal-move.html (rev 0)
+++ trunk/LayoutTests/fast/repaint/do-no-repaint-on-internal-move.html 2020-05-14 04:55:18 UTC (rev 261675)
@@ -0,0 +1,10 @@
+<span contenteditable="true"><span>Pass<audio controls="controls"></span>
+</span>if no crash.<div id=empty_div></div><script>
+document.execCommand("justifyFull", false);
+var range = document.createRange();
+range.setEndBefore(empty_div);
+window.getSelection().addRange(range);
+document.execCommand("justifyFull", false);
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
Modified: trunk/Source/WebCore/ChangeLog (261674 => 261675)
--- trunk/Source/WebCore/ChangeLog 2020-05-14 04:00:00 UTC (rev 261674)
+++ trunk/Source/WebCore/ChangeLog 2020-05-14 04:55:18 UTC (rev 261675)
@@ -1,3 +1,27 @@
+2020-05-13 Zalan Bujtas <[email protected]>
+
+ Do not clear selection/repaint when the renderer gets moved during tree normalization.
+ https://bugs.webkit.org/show_bug.cgi?id=211865
+ <rdar://problem/62849044>
+
+ Reviewed by Antti Koivisto.
+
+ While detachFromRenderElement should really be about "this renderer is being detached from its parent", some code in here assumes
+ the renderer is not only going to be detached but also going to be destroyed. Ideally such code should live in RenderObject::willBeDestroyed(),
+ but no tree walk is possible in there anymore.
+
+ The reason for the crash is that when we split the inline for continuation, we construct new anonymous containers and move subtrees over
+ to these new renderers. However at this point these new parent containers are not yet attached to the tree
+ so any tree-walking cleanup code will fail (in detachFromRenderElement).
+
+ Test: fast/repaint/do-no-repaint-on-internal-move.html
+
+ * rendering/updating/RenderTreeBuilder.cpp:
+ (WebCore::RenderTreeBuilder::detachFromRenderElement):
+ * rendering/updating/RenderTreeBuilder.h:
+ * rendering/updating/RenderTreeBuilderInline.cpp:
+ (WebCore::RenderTreeBuilder::Inline::splitInlines):
+
2020-05-13 Devin Rousso <[email protected]>
Web Inspector: show EventTarget listeners as an internal property
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp (261674 => 261675)
--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp 2020-05-14 04:00:00 UTC (rev 261674)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp 2020-05-14 04:55:18 UTC (rev 261675)
@@ -838,7 +838,7 @@
return takenChild;
}
-RenderPtr<RenderObject> RenderTreeBuilder::detachFromRenderElement(RenderElement& parent, RenderObject& child)
+RenderPtr<RenderObject> RenderTreeBuilder::detachFromRenderElement(RenderElement& parent, RenderObject& child, WillBeDestroyed willBeDestroyed)
{
RELEASE_ASSERT_WITH_MESSAGE(!parent.view().frameView().layoutContext().layoutState(), "Layout must not mutate render tree");
@@ -871,7 +871,7 @@
// If child is the start or end of the selection, then clear the selection to
// avoid problems of invalid pointers.
- if (!parent.renderTreeBeingDestroyed() && child.isSelectionBorder())
+ if (!parent.renderTreeBeingDestroyed() && willBeDestroyed == WillBeDestroyed::Yes && child.isSelectionBorder())
parent.frame().selection().setNeedsSelectionUpdate();
child.resetFragmentedFlowStateOnRemoval();
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.h (261674 => 261675)
--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.h 2020-05-14 04:00:00 UTC (rev 261674)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.h 2020-05-14 04:55:18 UTC (rev 261675)
@@ -71,7 +71,8 @@
void attachToRenderElement(RenderElement& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);
void attachToRenderElementInternal(RenderElement& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);
- RenderPtr<RenderObject> detachFromRenderElement(RenderElement& parent, RenderObject& child) WARN_UNUSED_RETURN;
+ enum class WillBeDestroyed { No, Yes };
+ RenderPtr<RenderObject> detachFromRenderElement(RenderElement& parent, RenderObject& child, WillBeDestroyed = WillBeDestroyed::Yes) WARN_UNUSED_RETURN;
RenderPtr<RenderObject> detachFromRenderGrid(RenderGrid& parent, RenderObject& child) WARN_UNUSED_RETURN;
void move(RenderBoxModelObject& from, RenderBoxModelObject& to, RenderObject& child, RenderObject* beforeChild, NormalizeAfterInsertion);
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp (261674 => 261675)
--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp 2020-05-14 04:00:00 UTC (rev 261674)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp 2020-05-14 04:55:18 UTC (rev 261675)
@@ -309,7 +309,7 @@
// FIXME: When the anonymous wrapper has multiple children, we end up traversing up to the topmost wrapper
// every time, which is a bit wasteful.
}
- auto childToMove = m_builder.detachFromRenderElement(*rendererToMove->parent(), *rendererToMove);
+ auto childToMove = m_builder.detachFromRenderElement(*rendererToMove->parent(), *rendererToMove, WillBeDestroyed::No);
m_builder.attachIgnoringContinuation(*cloneInline, WTFMove(childToMove));
rendererToMove->setNeedsLayoutAndPrefWidthsRecalc();
rendererToMove = nextSibling;
@@ -347,7 +347,7 @@
// *after* currentChild and append them all to the clone.
for (auto* sibling = currentChild->nextSibling(); sibling;) {
auto* next = sibling->nextSibling();
- auto childToMove = m_builder.detachFromRenderElement(*current, *sibling);
+ auto childToMove = m_builder.detachFromRenderElement(*current, *sibling, WillBeDestroyed::No);
m_builder.attachIgnoringContinuation(*cloneInline, WTFMove(childToMove));
sibling->setNeedsLayoutAndPrefWidthsRecalc();
sibling = next;
@@ -371,7 +371,7 @@
// and put them in the toBlock.
for (auto* current = currentChild->nextSibling(); current;) {
auto* next = current->nextSibling();
- auto childToMove = m_builder.detachFromRenderElement(*fromBlock, *current);
+ auto childToMove = m_builder.detachFromRenderElement(*fromBlock, *current, WillBeDestroyed::No);
m_builder.attachToRenderElementInternal(*toBlock, WTFMove(childToMove));
current = next;
}
