Skip to content

Commit

Permalink
Allow explicit inherit in matched declarations cache
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260199
rdar://113902520

Reviewed by Alan Baradlay.

If explicit inherit is involved we just need to apply it on top of the cached value.

* Source/WebCore/style/ElementRuleCollector.cpp:
(WebCore::Style::ElementRuleCollector::addMatchedProperties):
* Source/WebCore/style/PropertyCascade.cpp:
(WebCore::Style::PropertyCascade::addMatch):

Check for explicit inherit if needed.

* Source/WebCore/style/PropertyCascade.h:
* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::applyMatchedProperties):

Canonical link: https://commits.webkit.org/266986@main
  • Loading branch information
anttijk committed Aug 17, 2023
1 parent 147e066 commit b733d19
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 23 deletions.
5 changes: 0 additions & 5 deletions Source/WebCore/style/ElementRuleCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,12 +671,7 @@ void ElementRuleCollector::addMatchedProperties(MatchedProperties&& matchedPrope
if (current.isInherited())
continue;

// If the property value is explicitly inherited, we need to apply further non-inherited properties
// as they might override the value inherited here. For this reason we don't allow declarations with
// explicitly inherited properties to be cached.
auto& value = *current.value();
if (isValueID(value, CSSValueInherit))
return false;

// The value currentColor has implicitely the same side effect. It depends on the value of color,
// which is an inherited value, making the non-inherited property implicitly inherited.
Expand Down
28 changes: 17 additions & 11 deletions Source/WebCore/style/PropertyCascade.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,27 +221,33 @@ bool PropertyCascade::addMatch(const MatchedProperties& matchedProperties, Casca

if (m_includedProperties.containsAll(allProperties()))
return true;
if (m_includedProperties.contains(PropertyType::Inherited) && current.isInherited())
return true;
if (m_includedProperties.contains(PropertyType::NonInherited) && !current.isInherited())
return true;

// If we have applied this property for some reason already we must apply anything that overrides it.
if (hasProperty(propertyID, *current.value()))
return true;

if (m_includedProperties.contains(PropertyType::VariableReference)) {
if (current.value()->hasVariableReferences())
return true;
// Apply all deferred properties if we have applied any. They may override the ones we already applied.
if (propertyID >= firstDeferredProperty && m_lastIndexForDeferred)
return true;
}
if (m_includedProperties.containsAny({ PropertyType::AfterAnimation, PropertyType::AfterTransition })) {
if (shouldApplyAfterAnimation(current)) {
m_animationLayer->overriddenProperties.add(propertyID);
return true;
}
return false;
}

if (m_includedProperties.contains(PropertyType::Inherited) && current.isInherited())
return true;
if (m_includedProperties.contains(PropertyType::ExplicitlyInherited) && isValueID(*current.value(), CSSValueInherit))
return true;
if (m_includedProperties.contains(PropertyType::NonInherited) && !current.isInherited())
return true;

// Apply all deferred properties if we have applied any. They may override the ones we already applied.
if (propertyID >= firstDeferredProperty && m_lastIndexForDeferred)
return true;

if (m_includedProperties.contains(PropertyType::VariableReference)) {
if (current.value()->hasVariableReferences())
return true;
}

return false;
Expand Down
7 changes: 4 additions & 3 deletions Source/WebCore/style/PropertyCascade.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ class PropertyCascade {
enum class PropertyType : uint8_t {
NonInherited = 1 << 0,
Inherited = 1 << 1,
VariableReference = 1 << 2,
AfterAnimation = 1 << 3,
AfterTransition = 1 << 4
ExplicitlyInherited = 1 << 2,
VariableReference = 1 << 3,
AfterAnimation = 1 << 4,
AfterTransition = 1 << 5
};
static constexpr OptionSet<PropertyType> allProperties() { return { PropertyType::NonInherited, PropertyType::Inherited }; }

Expand Down
13 changes: 9 additions & 4 deletions Source/WebCore/style/StyleResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,10 @@ void Resolver::applyMatchedProperties(State& state, const MatchResult& matchResu
// element context. This is fast and saves memory by reusing the style data structures.
style.copyNonInheritedFrom(*cacheEntry->renderStyle);

if (parentStyle.inheritedEqual(*cacheEntry->parentRenderStyle) && !isAtShadowBoundary(element)) {
bool hasExplicitlyInherited = cacheEntry->renderStyle->hasExplicitlyInheritedProperties();
bool inheritedStyleEqual = parentStyle.inheritedEqual(*cacheEntry->parentRenderStyle) && !isAtShadowBoundary(element);

if (inheritedStyleEqual && !hasExplicitlyInherited) {
InsideLink linkStatus = state.style()->insideLink();
// If the cache item parent style has identical inherited properties to the current parent style then the
// resulting style will be identical too. We copy the inherited properties over from the cache and are done.
Expand All @@ -635,9 +638,11 @@ void Resolver::applyMatchedProperties(State& state, const MatchResult& matchResu
return;
}

includedProperties = { PropertyCascade::PropertyType::Inherited };

if (!parentStyle.inheritedCustomPropertiesEqual(*cacheEntry->parentRenderStyle))
if (!inheritedStyleEqual)
includedProperties.add(PropertyCascade::PropertyType::Inherited);
if (hasExplicitlyInherited)
includedProperties.add(PropertyCascade::PropertyType::ExplicitlyInherited);
if (!inheritedStyleEqual && !parentStyle.inheritedCustomPropertiesEqual(*cacheEntry->parentRenderStyle))
includedProperties.add(PropertyCascade::PropertyType::VariableReference);
}

Expand Down

0 comments on commit b733d19

Please sign in to comment.