Title: [290924] branches/safari-613-branch/Source/WebCore
Revision
290924
Author
[email protected]
Date
2022-03-07 14:09:30 -0800 (Mon, 07 Mar 2022)

Log Message

Cherry-pick r289814. rdar://problem/83101090

    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.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@289814 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (290923 => 290924)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-03-07 22:09:26 UTC (rev 290923)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-03-07 22:09:30 UTC (rev 290924)
@@ -1,5 +1,71 @@
 2022-03-07  Russell Epstein  <[email protected]>
 
+        Cherry-pick r289814. rdar://problem/83101090
+
+    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.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@289814 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-03-07  Russell Epstein  <[email protected]>
+
         Cherry-pick r289700. rdar://problem/85249360
 
     WebGL2 AllowShared TypedArray should be accepted

Modified: branches/safari-613-branch/Source/WebCore/rendering/LegacyRootInlineBox.cpp (290923 => 290924)


--- branches/safari-613-branch/Source/WebCore/rendering/LegacyRootInlineBox.cpp	2022-03-07 22:09:26 UTC (rev 290923)
+++ branches/safari-613-branch/Source/WebCore/rendering/LegacyRootInlineBox.cpp	2022-03-07 22:09:30 UTC (rev 290924)
@@ -75,7 +75,7 @@
 {
     detachEllipsisBox();
 
-    if (blockFlow().enclosingFragmentedFlow())
+    if (!renderer().document().renderTreeBeingDestroyed() && blockFlow().enclosingFragmentedFlow())
         containingFragmentMap(blockFlow()).remove(this);
 }
 

Modified: branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp (290923 => 290924)


--- branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp	2022-03-07 22:09:26 UTC (rev 290923)
+++ branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp	2022-03-07 22:09:30 UTC (rev 290924)
@@ -73,6 +73,7 @@
 #include "RenderTreeBuilderTable.h"
 #include "RenderTreeMutationDisallowedScope.h"
 #include "RenderView.h"
+#include <wtf/SetForScope.h>
 
 #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
 #include "FrameView.h"
@@ -443,12 +444,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);
 
@@ -483,8 +484,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 = [&] {
@@ -916,7 +918,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");
 
@@ -954,10 +956,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: branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.h (290923 => 290924)


--- branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.h	2022-03-07 22:09:26 UTC (rev 290923)
+++ branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilder.h	2022-03-07 22:09:30 UTC (rev 290924)
@@ -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: branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp (290923 => 290924)


--- branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp	2022-03-07 22:09:26 UTC (rev 290923)
+++ branches/safari-613-branch/Source/WebCore/rendering/updating/RenderTreeBuilderInline.cpp	2022-03-07 22:09:30 UTC (rev 290924)
@@ -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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to