Skip to content

Commit

Permalink
Cherry-pick 00414cb. rdar://123858279
Browse files Browse the repository at this point in the history
    Cherry-pick 274097.6@webkit-2024.2-embargoed (446b237f7e06). rdar://115001663

        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 Mohsin Qureshi committed Mar 6, 2024
1 parent 54236b0 commit 460ff24
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 @@ -362,7 +362,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 @@ -395,10 +395,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 460ff24

Please sign in to comment.