Skip to content
Permalink
Browse files
Disallow styles using container units from matched declarations cache
https://bugs.webkit.org/show_bug.cgi?id=241261

Reviewed by Alan Bujtas.

We may fail to invalidate styles using container units correctly on container size change
because they are getting cached.

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-units-invalidation-expected.txt:
* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::computeNonCalcLengthDouble):

Mark styles that use container units.

* Source/WebCore/css/CSSToLengthConversionData.cpp:
(WebCore::CSSToLengthConversionData::defaultViewportFactor const):
(WebCore::CSSToLengthConversionData::smallViewportFactor const):
(WebCore::CSSToLengthConversionData::largeViewportFactor const):
(WebCore::CSSToLengthConversionData::dynamicViewportFactor const):
(WebCore::CSSToLengthConversionData::setUsesContainerUnits const):
* Source/WebCore/css/CSSToLengthConversionData.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::updateViewportUnitsOnResize):
* Source/WebCore/rendering/RenderIFrame.cpp:
(WebCore::RenderIFrame::isFullScreenIFrame const):
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::RenderStyle):
* Source/WebCore/rendering/style/RenderStyle.h:
(WebCore::RenderStyle::setUsesViewportUnits):
(WebCore::RenderStyle::usesViewportUnits const):

Also rename hasViewportUnits -> usesViewportUnits for clarity.

(WebCore::RenderStyle::setUsesContainerUnits):
(WebCore::RenderStyle::usesContainerUnits const):
(WebCore::RenderStyle::NonInheritedFlags::operator== const):
(WebCore::RenderStyle::NonInheritedFlags::copyNonInheritedFrom):
(WebCore::RenderStyle::setHasViewportUnits): Deleted.
(WebCore::RenderStyle::hasViewportUnits const): Deleted.
* Source/WebCore/style/MatchedDeclarationsCache.cpp:
(WebCore::Style::MatchedDeclarationsCache::isCacheable):

Disallow caching.

(WebCore::Style::MatchedDeclarationsCache::clearEntriesAffectedByViewportUnits):
* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::styleForElement):
(WebCore::Style::Resolver::pseudoStyleForElement):

Canonical link: https://commits.webkit.org/251268@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295211 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
anttijk committed Jun 3, 2022
1 parent aded8ff commit 5ab0958877005562dbf271d95a298d0208e9d480
Showing 10 changed files with 37 additions and 20 deletions.
@@ -1,8 +1,8 @@
Test

FAIL cqi respond when selected container changes type (inline-size -> none) assert_equals: expected "50px" but got "30px"
FAIL cqb respond when selected container changes type (size -> none) assert_equals: expected "60px" but got "40px"
FAIL cqb respond when intermediate container changes type (inline-size -> size) assert_equals: expected "20px" but got "40px"
FAIL cqi respond when selected container changes inline-size assert_equals: expected "5px" but got "30px"
FAIL cqb respond when selected container changes block-size assert_equals: expected "5px" but got "40px"
PASS cqi respond when selected container changes type (inline-size -> none)
PASS cqb respond when selected container changes type (size -> none)
PASS cqb respond when intermediate container changes type (inline-size -> size)
PASS cqi respond when selected container changes inline-size
PASS cqb respond when selected container changes block-size

@@ -824,6 +824,7 @@ double CSSPrimitiveValue::computeUnzoomedNonCalcLengthDouble(CSSUnitType primiti
double CSSPrimitiveValue::computeNonCalcLengthDouble(const CSSToLengthConversionData& conversionData, CSSUnitType primitiveType, double value)
{
auto selectContainerRenderer = [&](CQ::Axis axis) -> const RenderBox* {
conversionData.setUsesContainerUnits();
if (!conversionData.element())
return nullptr;
// FIXME: Use cached query containers when available.
@@ -66,7 +66,7 @@ float CSSToLengthConversionData::zoom() const
FloatSize CSSToLengthConversionData::defaultViewportFactor() const
{
if (m_viewportDependencyDetectionStyle)
m_viewportDependencyDetectionStyle->setHasViewportUnits();
m_viewportDependencyDetectionStyle->setUsesViewportUnits();

if (!m_renderView)
return { };
@@ -77,7 +77,7 @@ FloatSize CSSToLengthConversionData::defaultViewportFactor() const
FloatSize CSSToLengthConversionData::smallViewportFactor() const
{
if (m_viewportDependencyDetectionStyle)
m_viewportDependencyDetectionStyle->setHasViewportUnits();
m_viewportDependencyDetectionStyle->setUsesViewportUnits();

if (!m_renderView)
return { };
@@ -88,7 +88,7 @@ FloatSize CSSToLengthConversionData::smallViewportFactor() const
FloatSize CSSToLengthConversionData::largeViewportFactor() const
{
if (m_viewportDependencyDetectionStyle)
m_viewportDependencyDetectionStyle->setHasViewportUnits();
m_viewportDependencyDetectionStyle->setUsesViewportUnits();

if (!m_renderView)
return { };
@@ -99,12 +99,18 @@ FloatSize CSSToLengthConversionData::largeViewportFactor() const
FloatSize CSSToLengthConversionData::dynamicViewportFactor() const
{
if (m_viewportDependencyDetectionStyle)
m_viewportDependencyDetectionStyle->setHasViewportUnits();
m_viewportDependencyDetectionStyle->setUsesViewportUnits();

if (!m_renderView)
return { };

return m_renderView->sizeForCSSDynamicViewportUnits() / 100.0;
}

void CSSToLengthConversionData::setUsesContainerUnits() const
{
if (m_viewportDependencyDetectionStyle)
m_viewportDependencyDetectionStyle->setUsesContainerUnits();
}

} // namespace WebCore
@@ -94,6 +94,8 @@ class CSSToLengthConversionData {
return copy;
}

void setUsesContainerUnits() const;

private:
const RenderStyle* m_style { nullptr };
const RenderStyle* m_rootStyle { nullptr };
@@ -4343,7 +4343,7 @@ void Document::updateViewportUnitsOnResize()
// FIXME: Ideally, we should save the list of elements that have viewport units and only iterate over those.
for (RefPtr element = ElementTraversal::firstWithin(rootNode()); element; element = ElementTraversal::nextIncludingPseudo(*element)) {
auto* renderer = element->renderer();
if (renderer && renderer->style().hasViewportUnits())
if (renderer && renderer->style().usesViewportUnits())
element->invalidateStyle();
}
}
@@ -76,7 +76,7 @@ bool RenderIFrame::isFullScreenIFrame() const
{
// Some authors implement fullscreen popups as out-of-flow iframes with size set to full viewport (using vw/vh units).
// The size used may not perfectly match the viewport size so the following heuristic uses a relaxed constraint.
return style().hasOutOfFlowPosition() && style().hasViewportUnits();
return style().hasOutOfFlowPosition() && style().usesViewportUnits();
}

bool RenderIFrame::flattenFrame() const
@@ -198,7 +198,8 @@ RenderStyle::RenderStyle(CreateDefaultStyleTag)
m_nonInheritedFlags.hasExplicitlySetDirection = false;
m_nonInheritedFlags.hasExplicitlySetWritingMode = false;
m_nonInheritedFlags.hasExplicitlySetTextAlign = false;
m_nonInheritedFlags.hasViewportUnits = false;
m_nonInheritedFlags.usesViewportUnits = false;
m_nonInheritedFlags.usesContainerUnits = false;
m_nonInheritedFlags.hasExplicitlyInheritedProperties = false;
m_nonInheritedFlags.disallowsFastPathInheritance = false;
m_nonInheritedFlags.isUnique = false;
@@ -197,8 +197,10 @@ class RenderStyle {
void setInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&&);
void setNonInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&&);

void setHasViewportUnits(bool v = true) { m_nonInheritedFlags.hasViewportUnits = v; }
bool hasViewportUnits() const { return m_nonInheritedFlags.hasViewportUnits; }
void setUsesViewportUnits() { m_nonInheritedFlags.usesViewportUnits = true; }
bool usesViewportUnits() const { return m_nonInheritedFlags.usesViewportUnits; }
void setUsesContainerUnits() { m_nonInheritedFlags.usesContainerUnits = true; }
bool usesContainerUnits() const { return m_nonInheritedFlags.usesContainerUnits; }

void setColumnStylesFromPaginationMode(const Pagination::Mode&);

@@ -1992,7 +1994,8 @@ class RenderStyle {
#if ENABLE(DARK_MODE_CSS)
unsigned hasExplicitlySetColorScheme : 1;
#endif
unsigned hasViewportUnits : 1;
unsigned usesViewportUnits : 1;
unsigned usesContainerUnits : 1;
unsigned hasExplicitlyInheritedProperties : 1; // Explicitly inherits a non-inherited property.
unsigned disallowsFastPathInheritance : 1;
unsigned isUnique : 1; // Style cannot be shared.
@@ -2133,7 +2136,8 @@ inline bool RenderStyle::NonInheritedFlags::operator==(const NonInheritedFlags&
#if ENABLE(DARK_MODE_CSS)
&& hasExplicitlySetColorScheme == other.hasExplicitlySetColorScheme
#endif
&& hasViewportUnits == other.hasViewportUnits
&& usesViewportUnits == other.usesViewportUnits
&& usesContainerUnits == other.usesContainerUnits
&& hasExplicitlyInheritedProperties == other.hasExplicitlyInheritedProperties
&& disallowsFastPathInheritance == other.disallowsFastPathInheritance
&& isUnique == other.isUnique
@@ -2159,7 +2163,8 @@ inline void RenderStyle::NonInheritedFlags::copyNonInheritedFrom(const NonInheri
unicodeBidi = other.unicodeBidi;
floating = other.floating;
tableLayout = other.tableLayout;
hasViewportUnits = other.hasViewportUnits;
usesViewportUnits = other.usesViewportUnits;
usesContainerUnits = other.usesContainerUnits;
hasExplicitlyInheritedProperties = other.hasExplicitlyInheritedProperties;
disallowsFastPathInheritance = other.disallowsFastPathInheritance;

@@ -61,6 +61,8 @@ bool MatchedDeclarationsCache::isCacheable(const Element& element, const RenderS
// The cache assumes static knowledge about which properties are inherited.
if (style.hasExplicitlyInheritedProperties())
return false;
if (style.usesContainerUnits())
return false;

// Getting computed style after a font environment change but before full style resolution may involve styles with non-current fonts.
// Avoid caching them.
@@ -138,7 +140,7 @@ void MatchedDeclarationsCache::invalidate()
void MatchedDeclarationsCache::clearEntriesAffectedByViewportUnits()
{
m_entries.removeIf([](auto& keyValue) {
return keyValue.value.renderStyle->hasViewportUnits();
return keyValue.value.renderStyle->usesViewportUnits();
});
}

@@ -269,7 +269,7 @@ ElementStyle Resolver::styleForElement(const Element& element, const ResolutionC
Adjuster adjuster(document(), *state.parentStyle(), context.parentBoxStyle, &element);
adjuster.adjust(*state.style(), state.userAgentAppearanceStyle());

if (state.style()->hasViewportUnits())
if (state.style()->usesViewportUnits())
document().setHasStyleWithViewportUnits();

return { state.takeStyle(), WTFMove(elementStyleRelations) };
@@ -465,7 +465,7 @@ std::unique_ptr<RenderStyle> Resolver::pseudoStyleForElement(const Element& elem
Adjuster adjuster(document(), *state.parentStyle(), context.parentBoxStyle, nullptr);
adjuster.adjust(*state.style(), state.userAgentAppearanceStyle());

if (state.style()->hasViewportUnits())
if (state.style()->usesViewportUnits())
document().setHasStyleWithViewportUnits();

return state.takeStyle();

0 comments on commit 5ab0958

Please sign in to comment.