Skip to content

Commit

Permalink
Invalidate rebuild root renderer on child node removal
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=265384
rdar://118834858

Reviewed by Alan Baradlay.

If a child of a rebuild root is removed we need to invalidate it for rebuild so it doesn't
get left is an inconsitent state.

Tested by

fast/ruby/rubyDOM-remove-rt2.html
fast/ruby/rubyDOM-remove-text2.html

with style based ruby enabled.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::invalidateRenderer):

Add a mechanism to invalidate just the renderer without requiring re-resolving style.

* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::invalidateStyle):
* Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:
(WebCore::findRenderingAncestor):
(WebCore::invalidateRebuildRootIfNeeded):
(WebCore::RenderTreeUpdater::tearDownRenderers):
(WebCore::RenderTreeUpdater::tearDownRenderersAfterSlotChange):
(WebCore::RenderTreeUpdater::tearDownRenderer):

Invalidate if we are removing a child of a rebuild root.

* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveElement):

Add resolution type for renderer rebuild that just grabs the existing style.

(WebCore::Style::TreeResolver::determineResolutionType):
(WebCore::Style::TreeResolver::resetDescendantStyleRelations):
* Source/WebCore/style/StyleTreeResolver.h:

Canonical link: https://commits.webkit.org/271202@main
  • Loading branch information
anttijk committed Nov 28, 2023
1 parent 7aa3c2d commit d04e837
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 7 deletions.
5 changes: 5 additions & 0 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,11 @@ void Element::invalidateStyleAndRenderersForSubtree()
Node::invalidateStyle(Style::Validity::SubtreeInvalid, Style::InvalidationMode::RebuildRenderer);
}

void Element::invalidateRenderer()
{
Node::invalidateStyle(Style::Validity::Valid, Style::InvalidationMode::RebuildRenderer);
}

void Element::invalidateStyleInternal()
{
Node::invalidateStyle(Style::Validity::ElementInvalid);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ class Element : public ContainerNode {
// custom (not style based) renderers. This is expensive and should be avoided.
// Elements newly added to the tree are also in this state.
void invalidateStyleAndRenderersForSubtree();
void invalidateRenderer();

void invalidateStyleInternal();
void invalidateStyleForAnimation();
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/dom/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,8 @@ void Node::invalidateStyle(Style::Validity validity, Style::InvalidationMode mod
if (document().inRenderTreeUpdate())
return;

setNodeFlag(NodeFlag::IsComputedStyleInvalidFlag);
if (validity != Style::Validity::Valid)
setNodeFlag(NodeFlag::IsComputedStyleInvalidFlag);

bool markAncestors = styleValidity() == Style::Validity::Valid || mode == Style::InvalidationMode::InsertedIntoAncestor;

Expand Down
20 changes: 19 additions & 1 deletion Source/WebCore/rendering/updating/RenderTreeUpdater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ RenderTreeUpdater::RenderTreeUpdater(Document& document, Style::PostResolutionCa

RenderTreeUpdater::~RenderTreeUpdater() = default;

static Element* findRenderingAncestor(ContainerNode& node)
static Element* findRenderingAncestor(Node& node)
{
for (auto& ancestor : composedTreeAncestors(node)) {
if (ancestor.renderer())
Expand Down Expand Up @@ -633,13 +633,27 @@ void RenderTreeUpdater::updateRenderViewStyle()
m_document.renderView()->setStyle(RenderStyle::clone(*m_styleUpdate->initialContainingBlockUpdate()));
}

static void invalidateRebuildRootIfNeeded(Node& node)
{
auto* ancestor = findRenderingAncestor(node);
if (!ancestor)
return;
if (!RenderTreeBuilder::isRebuildRootForChildren(*ancestor->renderer()))
return;
ancestor->invalidateRenderer();
}

void RenderTreeUpdater::tearDownRenderers(Element& root)
{
if (!root.renderer() && !root.hasDisplayContents())
return;
auto* view = root.document().renderView();
if (!view)
return;

RenderTreeBuilder builder(*view);
tearDownRenderers(root, TeardownType::Full, builder);
invalidateRebuildRootIfNeeded(root);
}

void RenderTreeUpdater::tearDownRenderersAfterSlotChange(Element& host)
Expand All @@ -650,17 +664,21 @@ void RenderTreeUpdater::tearDownRenderersAfterSlotChange(Element& host)
auto* view = host.document().renderView();
if (!view)
return;

RenderTreeBuilder builder(*view);
tearDownRenderers(host, TeardownType::FullAfterSlotChange, builder);
invalidateRebuildRootIfNeeded(host);
}

void RenderTreeUpdater::tearDownRenderer(Text& text)
{
auto* view = text.document().renderView();
if (!view)
return;

RenderTreeBuilder builder(*view);
tearDownTextRenderer(text, builder);
invalidateRebuildRootIfNeeded(text);
}

void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownType, RenderTreeBuilder& builder)
Expand Down
20 changes: 17 additions & 3 deletions Source/WebCore/style/StyleTreeResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,13 @@ auto TreeResolver::resolveElement(Element& element, const RenderStyle* existingS
if (!element.rendererIsEverNeeded() && !element.hasDisplayContents())
return { };

if (resolutionType == ResolutionType::RebuildUsingExisting) {
return {
ElementUpdate { RenderStyle::clonePtr(*existingStyle), Change::Renderer },
DescendantsToResolve::RebuildAllUsingExisting
};
}

auto resolutionContext = makeResolutionContext();

Styleable styleable { element, PseudoId::None };
Expand Down Expand Up @@ -743,15 +750,21 @@ auto TreeResolver::determineResolutionType(const Element& element, const RenderS
return validity;
}();

if (combinedValidity == Validity::AnimationInvalid && parentDescendantsToResolve == DescendantsToResolve::None)
return ResolutionType::AnimationOnly;
if (parentDescendantsToResolve == DescendantsToResolve::None) {
if (combinedValidity == Validity::AnimationInvalid)
return ResolutionType::AnimationOnly;
if (combinedValidity == Validity::Valid && element.hasInvalidRenderer())
return existingStyle ? ResolutionType::RebuildUsingExisting : ResolutionType::Full;
}

if (combinedValidity != Validity::Valid)
if (combinedValidity > Validity::Valid)
return ResolutionType::Full;

switch (parentDescendantsToResolve) {
case DescendantsToResolve::None:
return { };
case DescendantsToResolve::RebuildAllUsingExisting:
return existingStyle ? ResolutionType::RebuildUsingExisting : ResolutionType::Full;
case DescendantsToResolve::Children:
if (parentChange == Change::FastPathInherited) {
if (existingStyle && !existingStyle->disallowsFastPathInheritance())
Expand Down Expand Up @@ -809,6 +822,7 @@ void TreeResolver::resetDescendantStyleRelations(Element& element, DescendantsTo
{
switch (descendantsToResolve) {
case DescendantsToResolve::None:
case DescendantsToResolve::RebuildAllUsingExisting:
case DescendantsToResolve::ChildrenWithExplicitInherit:
break;
case DescendantsToResolve::Children:
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/style/StyleTreeResolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ class TreeResolver {
bool hasUnresolvedQueryContainers() const { return m_hasUnresolvedQueryContainers; }

private:
enum class ResolutionType : uint8_t { AnimationOnly, FastPathInherit, Full };
enum class ResolutionType : uint8_t { RebuildUsingExisting, AnimationOnly, FastPathInherit, Full };
ResolvedStyle styleForStyleable(const Styleable&, ResolutionType, const ResolutionContext&);

void resolveComposedTree();

const RenderStyle* existingStyle(const Element&);

enum class QueryContainerAction : uint8_t { None, Resolve, Continue };
enum class DescendantsToResolve : uint8_t { None, ChildrenWithExplicitInherit, Children, All };
enum class DescendantsToResolve : uint8_t { None, RebuildAllUsingExisting, ChildrenWithExplicitInherit, Children, All };

QueryContainerAction updateStateForQueryContainer(Element&, const RenderStyle*, Change&, DescendantsToResolve&);

Expand Down

0 comments on commit d04e837

Please sign in to comment.