Skip to content

Commit

Permalink
Cherry-pick 274097.6@webkit-2024.2-embargoed (446b237f7e06). rdar://1…
Browse files Browse the repository at this point in the history
…15001663

    Prevent selection repaint in the middle of multicolumn flow destruction
    https://bugs.webkit.org/show_bug.cgi?id=263180

    Reviewed by Alan Baradlay.

    During multicolumn fragmented flow destruction, spanners are moved back
    to their original DOM position in the tree. This is done through calls
    to RenderTreeBuilderBlock::Block::detach(RenderBlockFlow&), which also
    calls the more general RenderBlock ::detach() method for each spanner.
    The former method results in the destruction of the spanner placeholders
    and the merging of the necessary multicolumn sets, but this is not done
    immediately, so the tree is temporarily inconsistent, before the
    RenderBlock detach() method is called.

    RenderTreeBuilderBlock::Block::detach(RenderBlock&), however,
    might inadvertely end up triggering a repaint of the selection that the
    tree is not ready for. I assume that this is an oversight from the possibility
    that this method gets called during RenderBlockFlow detachment. This repaint
    happens because RenderTreeBuilder::detachFromRenderElement() clears the
    selection if the child being detached is to be destroyed. As
    WillBeDestroyed::Yes is the default value in the definition of
    detachFromRenderElement(), this is assumed to be the case, even when
    that's not what happens during fragmented flow destruction.

    The problem with this is that the selection repaint will eventually find itself
    needing a consistent tree, and the fact that multicolumn sets are not merged
    yet and there are spanners without a placehoder will break assumptions made
    in RenderObject::propagateRepaintToParentWithOutlineAutoIfNeeded().

    Fix this by making it possible for both detach() methods to propagate
    WillBeDestroyed, with a default value of WillBeDestroyed::Yes to preserve
    current behavior everywhere, but explicitly passing WillBeDestroyed::No
    during fragmented flow destruction when detaching spanners, as this is what
    is actually happening. This prevents the selection repaint from happening
    before the tree is in a consistent state.

    * LayoutTests/fast/block/multicolumn-with-outline-auto-expected.txt: Added.
    * LayoutTests/fast/block/multicolumn-with-outline-auto.html: Added.
    * Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:
    (WebCore::RenderTreeBuilder::detach):
    * Source/WebCore/rendering/updating/RenderTreeBuilder.h:
    * Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp:
    (WebCore::RenderTreeBuilder::Block::detach):
    * Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h:
    * Source/WebCore/rendering/updating/RenderTreeBuilderMultiColumn.cpp:
    (WebCore::RenderTreeBuilder::MultiColumn::destroyFragmentedFlow):

    Canonical link: https://commits.webkit.org/274097.6@webkit-2024.2-embargoed

Canonical link: https://commits.webkit.org/272448.648@safari-7618-branch
  • Loading branch information
csaavedra authored and JonWBedard committed Feb 29, 2024
1 parent 29dc10b commit 00414cb
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
HOCUS
POCUS
24 changes: 24 additions & 0 deletions LayoutTests/fast/block/multicolumn-with-outline-auto.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<style>
* { rotate: 180deg; outline-style: auto; column-span: all;}
*:focus-within { columns: 2; }
</style>
<script>
function f0() {
window.find("POCUS");
keygen.replaceWith("HOCUS");

if (window.testRunner)
testRunner.notifyDone();
}

if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}
</script>
<keygen id="keygen" onfocusin="f0()" autofocus="" contenteditable="true"/>
<dir>
<div id="div">
<label id="label">POCUS</label>
</div>
</dir>
6 changes: 3 additions & 3 deletions Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ void RenderTreeBuilder::attachIgnoringContinuation(RenderElement& parent, Render
attachInternal(parent, WTFMove(child), beforeChild);
}

RenderPtr<RenderObject> RenderTreeBuilder::detach(RenderElement& parent, RenderObject& child, CanCollapseAnonymousBlock canCollapseAnonymousBlock)
RenderPtr<RenderObject> RenderTreeBuilder::detach(RenderElement& parent, RenderObject& child, CanCollapseAnonymousBlock canCollapseAnonymousBlock, WillBeDestroyed willBeDestroyed)
{
if (auto* rubyInline = dynamicDowncast<RenderRubyAsInline>(parent))
return rubyBuilder().detach(*rubyInline, child);
Expand Down Expand Up @@ -413,10 +413,10 @@ RenderPtr<RenderObject> RenderTreeBuilder::detach(RenderElement& parent, RenderO
return svgBuilder().detach(*svgRoot, child);

if (auto* blockFlow = dynamicDowncast<RenderBlockFlow>(parent))
return blockBuilder().detach(*blockFlow, child, canCollapseAnonymousBlock);
return blockBuilder().detach(*blockFlow, child, canCollapseAnonymousBlock, willBeDestroyed);

if (auto* block = dynamicDowncast<RenderBlock>(parent))
return blockBuilder().detach(*block, child, canCollapseAnonymousBlock);
return blockBuilder().detach(*block, child, canCollapseAnonymousBlock, willBeDestroyed);

return detachFromRenderElement(parent, child);
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/updating/RenderTreeBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class RenderTreeBuilder {
void attach(RenderElement& parent, RenderPtr<RenderObject>, RenderObject* beforeChild = nullptr);

enum class CanCollapseAnonymousBlock : bool { No, Yes };
RenderPtr<RenderObject> detach(RenderElement&, RenderObject&, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes) WARN_UNUSED_RETURN;
enum class WillBeDestroyed : bool { No, Yes };
RenderPtr<RenderObject> detach(RenderElement&, RenderObject&, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes, WillBeDestroyed = WillBeDestroyed::Yes) WARN_UNUSED_RETURN;

void destroy(RenderObject& renderer, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes);

Expand All @@ -73,7 +74,6 @@ class RenderTreeBuilder {
void attachToRenderElement(RenderElement& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);
void attachToRenderElementInternal(RenderElement& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr, RenderObject::IsInternalMove = RenderObject::IsInternalMove::No);

enum class WillBeDestroyed : bool { 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;

Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ void RenderTreeBuilder::Block::removeLeftoverAnonymousBlock(RenderBlock& anonymo
// anonymousBlock is dead here.
}

RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlock& parent, RenderObject& oldChild, CanCollapseAnonymousBlock canCollapseAnonymousBlock)
RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlock& parent, RenderObject& oldChild, CanCollapseAnonymousBlock canCollapseAnonymousBlock, WillBeDestroyed willBeDestroyed)
{
// No need to waste time in merging or removing empty anonymous blocks.
// We can just bail out if our document is getting destroyed.
Expand All @@ -295,7 +295,7 @@ RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlock& parent, Re
WeakPtr next = oldChild.nextSibling();
bool canMergeAnonymousBlocks = canCollapseAnonymousBlock == CanCollapseAnonymousBlock::Yes && canMergeContiguousAnonymousBlocks(oldChild, prev.get(), next.get());

auto takenChild = m_builder.detachFromRenderElement(parent, oldChild);
auto takenChild = m_builder.detachFromRenderElement(parent, oldChild, willBeDestroyed);

if (canMergeAnonymousBlocks && prev && next) {
prev->setNeedsLayoutAndPrefWidthsRecalc();
Expand Down Expand Up @@ -382,14 +382,14 @@ void RenderTreeBuilder::Block::dropAnonymousBoxChild(RenderBlock& parent, Render
child.deleteLines();
}

RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlockFlow& parent, RenderObject& child, CanCollapseAnonymousBlock canCollapseAnonymousBlock)
RenderPtr<RenderObject> RenderTreeBuilder::Block::detach(RenderBlockFlow& parent, RenderObject& child, CanCollapseAnonymousBlock canCollapseAnonymousBlock, WillBeDestroyed willBeDestroyed)
{
if (!parent.renderTreeBeingDestroyed()) {
auto* fragmentedFlow = parent.multiColumnFlow();
if (fragmentedFlow && fragmentedFlow != &child)
m_builder.multiColumnBuilder().multiColumnRelativeWillBeRemoved(*fragmentedFlow, child, canCollapseAnonymousBlock);
}
return detach(static_cast<RenderBlock&>(parent), child, canCollapseAnonymousBlock);
return detach(static_cast<RenderBlock&>(parent), child, canCollapseAnonymousBlock, willBeDestroyed);
}

}
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/updating/RenderTreeBuilderBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class RenderTreeBuilder::Block {
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, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes) WARN_UNUSED_RETURN;
RenderPtr<RenderObject> detach(RenderBlockFlow& parent, RenderObject& child, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes) WARN_UNUSED_RETURN;
RenderPtr<RenderObject> detach(RenderBlock& parent, RenderObject& oldChild, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes, WillBeDestroyed = WillBeDestroyed::Yes) WARN_UNUSED_RETURN;
RenderPtr<RenderObject> detach(RenderBlockFlow& parent, RenderObject& child, CanCollapseAnonymousBlock = CanCollapseAnonymousBlock::Yes, WillBeDestroyed = WillBeDestroyed::Yes) WARN_UNUSED_RETURN;

void dropAnonymousBoxChild(RenderBlock& parent, RenderBlock& child);
void childBecameNonInline(RenderBlock& parent, RenderElement& child);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ void RenderTreeBuilder::MultiColumn::destroyFragmentedFlow(RenderBlockFlow& flow
spannerOriginalParent = &flow;
// Detaching the spanner takes care of removing the placeholder (and merges the RenderMultiColumnSets).
auto* spanner = placeholder->spanner();
parentAndSpannerList.append(std::make_pair(spannerOriginalParent, m_builder.detach(*spanner->parent(), *spanner, CanCollapseAnonymousBlock::No)));
parentAndSpannerList.append(std::make_pair(spannerOriginalParent, m_builder.detach(*spanner->parent(), *spanner, CanCollapseAnonymousBlock::No, WillBeDestroyed::No)));
}
while (auto* columnSet = multiColumnFlow.firstMultiColumnSet())
m_builder.destroy(*columnSet);
Expand Down

0 comments on commit 00414cb

Please sign in to comment.