- Revision
- 289814
- Author
- [email protected]
- Date
- 2022-02-15 07:42:32 -0800 (Tue, 15 Feb 2022)
Log Message
Do not update the fragmented flow state while internally mutating the render tree
https://bugs.webkit.org/show_bug.cgi?id=230896
Reviewed by Darin Adler.
RenderTree mutations (like those happening when creating/destroying anonymous blocks)
should not affect the fragment state of any renderer. This means that we should not have
to deal with things like creating/restoring placeholders/spanners while doing that.
There is already a IsInternalMove flag that is being used for that. Expand its usage
to a couple more methods to improve correctness.
* rendering/LegacyRootInlineBox.cpp:
(WebCore::LegacyRootInlineBox::~LegacyRootInlineBox): Do not remove the inline box from
the ContainingFragmentMap if we're deleting the tree. It was causing ASSERTs trying to
retrieve the enclosing fragmented flow in some cases.
* rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::attachToRenderElementInternal): Use the RenderTreeBuilder's
m_internalTreeBuilding instead of the argument.
(WebCore::RenderTreeBuilder::move): Replace passing the IsInternalMove argument by a
scope where we don't update the fragmented flow state.
(WebCore::RenderTreeBuilder::detachFromRenderElement): Use the RenderTreeBuilder's
m_internalMovesType instead of the argument.
* rendering/updating/RenderTreeBuilder.h:
* rendering/updating/RenderTreeBuilderInline.cpp:
(WebCore::RenderTreeBuilder::Inline::splitInlines): Wrap the method by a scope in which
fragmented flow state is not updated because we consider those operations internal arrangements
of the tree.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (289813 => 289814)
--- trunk/Source/WebCore/ChangeLog 2022-02-15 15:22:01 UTC (rev 289813)
+++ trunk/Source/WebCore/ChangeLog 2022-02-15 15:42:32 UTC (rev 289814)
@@ -1,3 +1,34 @@
+2022-02-10 Sergio Villar Senin <[email protected]>
+
+ Do not update the fragmented flow state while internally mutating the render tree
+ https://bugs.webkit.org/show_bug.cgi?id=230896
+
+ Reviewed by Darin Adler.
+
+ RenderTree mutations (like those happening when creating/destroying anonymous blocks)
+ should not affect the fragment state of any renderer. This means that we should not have
+ to deal with things like creating/restoring placeholders/spanners while doing that.
+
+ There is already a IsInternalMove flag that is being used for that. Expand its usage
+ to a couple more methods to improve correctness.
+
+ * rendering/LegacyRootInlineBox.cpp:
+ (WebCore::LegacyRootInlineBox::~LegacyRootInlineBox): Do not remove the inline box from
+ the ContainingFragmentMap if we're deleting the tree. It was causing ASSERTs trying to
+ retrieve the enclosing fragmented flow in some cases.
+ * rendering/updating/RenderTreeBuilder.cpp:
+ (WebCore::RenderTreeBuilder::attachToRenderElementInternal): Use the RenderTreeBuilder's
+ m_internalTreeBuilding instead of the argument.
+ (WebCore::RenderTreeBuilder::move): Replace passing the IsInternalMove argument by a
+ scope where we don't update the fragmented flow state.
+ (WebCore::RenderTreeBuilder::detachFromRenderElement): Use the RenderTreeBuilder's
+ m_internalMovesType instead of the argument.
+ * rendering/updating/RenderTreeBuilder.h:
+ * rendering/updating/RenderTreeBuilderInline.cpp:
+ (WebCore::RenderTreeBuilder::Inline::splitInlines): Wrap the method by a scope in which
+ fragmented flow state is not updated because we consider those operations internal arrangements
+ of the tree.
+
2022-02-15 Ziran Sun <[email protected]>
[Forms] the select() method returns should be in line with specs
Modified: trunk/Source/WebCore/rendering/LegacyRootInlineBox.cpp (289813 => 289814)
--- trunk/Source/WebCore/rendering/LegacyRootInlineBox.cpp 2022-02-15 15:22:01 UTC (rev 289813)
+++ trunk/Source/WebCore/rendering/LegacyRootInlineBox.cpp 2022-02-15 15:42:32 UTC (rev 289814)
@@ -75,7 +75,7 @@
{
detachEllipsisBox();
- if (blockFlow().enclosingFragmentedFlow())
+ if (!renderer().document().renderTreeBeingDestroyed() && blockFlow().enclosingFragmentedFlow())
containingFragmentMap(blockFlow()).remove(this);
}
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp (289813 => 289814)
--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp 2022-02-15 15:22:01 UTC (rev 289813)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp 2022-02-15 15:42:32 UTC (rev 289814)
@@ -74,6 +74,7 @@
#include "RenderTreeBuilderTable.h"
#include "RenderTreeMutationDisallowedScope.h"
#include "RenderView.h"
+#include <wtf/SetForScope.h>
#if ENABLE(LAYOUT_FORMATTING_CONTEXT)
#include "FrameView.h"
@@ -456,12 +457,12 @@
// Take the ownership.
auto* newChild = parent.attachRendererInternal(WTFMove(child), beforeChild);
- newChild->initializeFragmentedFlowStateOnInsertion();
+ if (m_internalMovesType == RenderObject::IsInternalMove::No)
+ newChild->initializeFragmentedFlowStateOnInsertion();
if (!parent.renderTreeBeingDestroyed()) {
newChild->insertedIntoTree(isInternalMove);
- auto needsStateReset = isInternalMove == RenderObject::IsInternalMove::No;
- if (needsStateReset) {
+ if (m_internalMovesType == RenderObject::IsInternalMove::No) {
if (auto* fragmentedFlow = newChild->enclosingFragmentedFlow(); is<RenderMultiColumnFlow>(fragmentedFlow))
multiColumnBuilder().multiColumnDescendantInserted(downcast<RenderMultiColumnFlow>(*fragmentedFlow), *newChild);
@@ -496,8 +497,9 @@
auto childToMove = detachFromRenderElement(from, child);
attach(to, WTFMove(childToMove), beforeChild);
} else {
- auto childToMove = detachFromRenderElement(from, child, WillBeDestroyed::No, RenderObject::IsInternalMove::Yes);
- attachToRenderElementInternal(to, WTFMove(childToMove), beforeChild, RenderObject::IsInternalMove::Yes);
+ auto internalMoveScope = SetForScope { m_internalMovesType, RenderObject::IsInternalMove::Yes };
+ auto childToMove = detachFromRenderElement(from, child, WillBeDestroyed::No);
+ attachToRenderElementInternal(to, WTFMove(childToMove), beforeChild);
}
auto findBFCRootAndDestroyInlineTree = [&] {
@@ -943,7 +945,7 @@
return takenChild;
}
-RenderPtr<RenderObject> RenderTreeBuilder::detachFromRenderElement(RenderElement& parent, RenderObject& child, WillBeDestroyed willBeDestroyed, RenderObject::IsInternalMove isInternalMove)
+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");
@@ -981,10 +983,11 @@
if (!parent.renderTreeBeingDestroyed() && willBeDestroyed == WillBeDestroyed::Yes && child.isSelectionBorder())
parent.frame().selection().setNeedsSelectionUpdate();
- child.resetFragmentedFlowStateOnRemoval();
+ if (!parent.renderTreeBeingDestroyed() && m_internalMovesType == RenderObject::IsInternalMove::No)
+ child.resetFragmentedFlowStateOnRemoval();
if (!parent.renderTreeBeingDestroyed())
- child.willBeRemovedFromTree(isInternalMove);
+ child.willBeRemovedFromTree(m_internalMovesType);
// WARNING: There should be no code running between willBeRemovedFromTree() and the actual removal below.
// This is needed to avoid race conditions where willBeRemovedFromTree() would dirty the tree's structure
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.h (289813 => 289814)
--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.h 2022-02-15 15:22:01 UTC (rev 289813)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.h 2022-02-15 15:42:32 UTC (rev 289814)
@@ -77,7 +77,7 @@
void attachToRenderElementInternal(RenderElement& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr, RenderObject::IsInternalMove = RenderObject::IsInternalMove::No);
enum class WillBeDestroyed { No, Yes };
- RenderPtr<RenderObject> detachFromRenderElement(RenderElement& parent, RenderObject& child, WillBeDestroyed = WillBeDestroyed::Yes, RenderObject::IsInternalMove = RenderObject::IsInternalMove::No) WARN_UNUSED_RETURN;
+ 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);
@@ -158,6 +158,7 @@
std::unique_ptr<FullScreen> m_fullScreenBuilder;
#endif
bool m_hasBrokenContinuation { false };
+ RenderObject::IsInternalMove m_internalMovesType { RenderObject::IsInternalMove::No };
};
}
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp (289813 => 289814)
--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp 2022-02-15 15:22:01 UTC (rev 289813)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp 2022-02-15 15:42:32 UTC (rev 289814)
@@ -34,6 +34,7 @@
#include "RenderTable.h"
#include "RenderTreeBuilderMultiColumn.h"
#include "RenderTreeBuilderTable.h"
+#include <wtf/SetForScope.h>
namespace WebCore {
@@ -266,6 +267,7 @@
void RenderTreeBuilder::Inline::splitInlines(RenderInline& parent, RenderBlock* fromBlock, RenderBlock* toBlock, RenderBlock* middleBlock, RenderObject* beforeChild, RenderBoxModelObject* oldCont)
{
+ auto internalMoveScope = SetForScope { m_builder.m_internalMovesType, RenderObject::IsInternalMove::Yes };
// Create a clone of this inline.
RenderPtr<RenderInline> cloneInline = cloneAsContinuation(parent);
#if ENABLE(FULLSCREEN_API)
