Skip to content

Commit

Permalink
[@Property] Handle dynamic updates to viewport units
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=255689
rdar://108287215

Reviewed by Antti Koivisto.

Viewport units are "computationally independent", and so are allowed in
registered custom property initial values. But we don't have any
mechanism to invalidate them when the viewport size changes.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::collectComputedStyleDependencies const):

Record whether the value had any viewport units.

* Source/WebCore/css/CSSRegisteredCustomProperty.h:

Store the CSS parser tokens an initial value was created from if it
it contains viewport units, so that we can re-parse it when the viewport
is resized.

* Source/WebCore/css/CSSValue.h:
(WebCore::ComputedStyleDependencies::isComputationallyIndependent const):
(WebCore::ComputedStyleDependencies::isEmpty const): Deleted.

Rename isEmpty, since we don't want it to look at the new
viewportDimensions member.

* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::ComputedStyleExtractor::customPropertyValue const):
(WebCore::ComputedStyleExtractor::propertyValue const):

Flush layout in the parent document if there are viewport units used
anywhere in the current document.

* Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp:
(WebCore::DOMCSSRegisterCustomProperty::registerProperty):
* Source/WebCore/style/CustomPropertyRegistry.cpp:
(WebCore::Style::CustomPropertyRegistry::registerFromAPI):
(WebCore::Style::CustomPropertyRegistry::registerFromStylesheet):
(WebCore::Style::CustomPropertyRegistry::parseInitialValue):

Factor out initial value parsing from
DOMCSSRegisterCustomProperty::registerProperty and
CustomPropertyRegistry::registerFromStylesheet into a new function
parseInitialValue, which returns both the newly parsed
CSSCustomPropertyValue as well as whether it contained viewport unit
values.

* Source/WebCore/style/CustomPropertyRegistry.h:
(WebCore::Style::CustomPropertyRegistry::invalidatePropertiesWithViewportUnits):
* Source/WebCore/style/StyleScope.cpp:
(WebCore::Style::Scope::didChangeViewportSize):

Respond to viewport size changes by invalidating all registered custom
properties whose initial value contains viewport units.

Canonical link: https://commits.webkit.org/267590@main
  • Loading branch information
heycam committed Sep 2, 2023
1 parent 138a647 commit bb27e4a
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 68 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@


FAIL @property: viewport units in initial value (dynamic) assert_equals: expected "10px" but got "40px"
PASS @property: viewport units in initial value (dynamic)

38 changes: 20 additions & 18 deletions Source/WebCore/css/CSSPrimitiveValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1701,24 +1701,6 @@ void CSSPrimitiveValue::collectComputedStyleDependencies(ComputedStyleDependenci
case CSSUnitType::CSS_CALC:
m_value.calc->collectComputedStyleDependencies(dependencies);
break;
case CSSUnitType::CSS_NUMBER:
case CSSUnitType::CSS_INTEGER:
case CSSUnitType::CSS_PERCENTAGE:
case CSSUnitType::CSS_PX:
case CSSUnitType::CSS_CM:
case CSSUnitType::CSS_MM:
case CSSUnitType::CSS_IN:
case CSSUnitType::CSS_PT:
case CSSUnitType::CSS_PC:
case CSSUnitType::CSS_DEG:
case CSSUnitType::CSS_RAD:
case CSSUnitType::CSS_GRAD:
case CSSUnitType::CSS_TURN:
case CSSUnitType::CSS_MS:
case CSSUnitType::CSS_S:
case CSSUnitType::CSS_HZ:
case CSSUnitType::CSS_KHZ:
case CSSUnitType::CSS_DIMENSION:
case CSSUnitType::CSS_VW:
case CSSUnitType::CSS_VH:
case CSSUnitType::CSS_VMIN:
Expand All @@ -1743,6 +1725,26 @@ void CSSPrimitiveValue::collectComputedStyleDependencies(ComputedStyleDependenci
case CSSUnitType::CSS_DVMAX:
case CSSUnitType::CSS_DVB:
case CSSUnitType::CSS_DVI:
dependencies.viewportDimensions = true;
break;
case CSSUnitType::CSS_NUMBER:
case CSSUnitType::CSS_INTEGER:
case CSSUnitType::CSS_PERCENTAGE:
case CSSUnitType::CSS_PX:
case CSSUnitType::CSS_CM:
case CSSUnitType::CSS_MM:
case CSSUnitType::CSS_IN:
case CSSUnitType::CSS_PT:
case CSSUnitType::CSS_PC:
case CSSUnitType::CSS_DEG:
case CSSUnitType::CSS_RAD:
case CSSUnitType::CSS_GRAD:
case CSSUnitType::CSS_TURN:
case CSSUnitType::CSS_MS:
case CSSUnitType::CSS_S:
case CSSUnitType::CSS_HZ:
case CSSUnitType::CSS_KHZ:
case CSSUnitType::CSS_DIMENSION:
case CSSUnitType::CSS_DPPX:
case CSSUnitType::CSS_X:
case CSSUnitType::CSS_DPI:
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/css/CSSRegisteredCustomProperty.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
namespace WebCore {

class CSSCustomPropertyValue;
class CSSVariableData;

struct CSSRegisteredCustomProperty {
WTF_MAKE_STRUCT_FAST_ALLOCATED;
Expand All @@ -39,6 +40,7 @@ struct CSSRegisteredCustomProperty {
CSSCustomPropertySyntax syntax;
bool inherits;
RefPtr<const CSSCustomPropertyValue> initialValue;
RefPtr<const CSSVariableData> initialValueTokensForViewportUnits;

~CSSRegisteredCustomProperty();
};
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/css/CSSValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ struct ComputedStyleDependencies {
Vector<CSSPropertyID> properties;
Vector<CSSPropertyID> rootProperties;
bool containerDimensions { false };
bool viewportDimensions { false };

bool isEmpty() const { return properties.isEmpty() && rootProperties.isEmpty() && !containerDimensions; }
bool isComputationallyIndependent() const { return properties.isEmpty() && rootProperties.isEmpty() && !containerDimensions; }
};

DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(CSSValue);
Expand Down
43 changes: 33 additions & 10 deletions Source/WebCore/css/ComputedStyleExtractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include "FontCascade.h"
#include "FontSelectionValueInlines.h"
#include "GridPositionsResolver.h"
#include "HTMLFrameOwnerElement.h"
#include "MotionPath.h"
#include "NodeRenderStyle.h"
#include "PerspectiveTransformOperation.h"
Expand Down Expand Up @@ -2799,6 +2800,15 @@ RefPtr<CSSValue> ComputedStyleExtractor::customPropertyValue(const AtomString& p
if (!style)
return nullptr;

auto& document = styledElement->document();

if (document.hasStyleWithViewportUnits()) {
if (RefPtr owner = document.ownerElement()) {
owner->document().updateLayout();
style = computeRenderStyleForProperty(*styledElement, m_pseudoElementSpecifier, CSSPropertyCustom, ownedStyle, nullptr);
}
}

auto* value = style->customPropertyValue(propertyName);

return const_cast<CSSCustomPropertyValue*>(value);
Expand Down Expand Up @@ -2853,6 +2863,8 @@ static Ref<CSSFontValue> fontShorthandValue(const RenderStyle& style, ComputedSt
return computedFont;
}

enum class ForcedLayout : uint8_t { No, Yes, ParentDocument };

RefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID, UpdateLayout updateLayout, PropertyValueType valueType) const
{
auto* styledElement = m_element.get();
Expand All @@ -2869,7 +2881,8 @@ RefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID,

std::unique_ptr<RenderStyle> ownedStyle;
const RenderStyle* style = nullptr;
bool forceFullLayout = false;
auto forcedLayout = ForcedLayout::No;

if (updateLayout == UpdateLayout::Yes) {
Document& document = m_element->document();

Expand All @@ -2879,26 +2892,36 @@ RefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID,

style = computeRenderStyleForProperty(*styledElement, m_pseudoElementSpecifier, propertyID, ownedStyle, styledRenderer());

forceFullLayout = [&] {
forcedLayout = [&] {
// FIXME: Some of these cases could be narrowed down or optimized better.
if (isLayoutDependent(propertyID, style, styledRenderer()))
return true;
return ForcedLayout::Yes;
// FIXME: Why?
if (styledElement->isInShadowTree())
return true;
return ForcedLayout::Yes;
if (!document.ownerElement())
return false;
return ForcedLayout::No;
if (!document.styleScope().resolverIfExists())
return false;
auto& ruleSets = document.styleScope().resolverIfExists()->ruleSets();
return ruleSets.hasViewportDependentMediaQueries() || ruleSets.hasContainerQueries();
return ForcedLayout::No;
if (auto& ruleSets = document.styleScope().resolverIfExists()->ruleSets(); ruleSets.hasViewportDependentMediaQueries() || ruleSets.hasContainerQueries())
return ForcedLayout::Yes;
// FIXME: Can we limit this to properties whose computed length value derived from a viewport unit?
if (document.hasStyleWithViewportUnits())
return ForcedLayout::ParentDocument;
return ForcedLayout::No;
}();

if (forceFullLayout)
if (forcedLayout == ForcedLayout::Yes)
document.updateLayoutIgnorePendingStylesheets();
else if (forcedLayout == ForcedLayout::ParentDocument) {
if (RefPtr owner = document.ownerElement())
owner->document().updateLayout();
else
forcedLayout = ForcedLayout::No;
}
}

if (updateLayout == UpdateLayout::No || forceFullLayout)
if (updateLayout == UpdateLayout::No || forcedLayout != ForcedLayout::No)
style = computeRenderStyleForProperty(*styledElement, m_pseudoElementSpecifier, propertyID, ownedStyle, styledRenderer());

if (!style)
Expand Down
29 changes: 18 additions & 11 deletions Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
namespace WebCore {
DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(DOMCSSRegisterCustomProperty);

using namespace Style;

ExceptionOr<void> DOMCSSRegisterCustomProperty::registerProperty(Document& document, const DOMCSSCustomPropertyDescriptor& descriptor)
{
if (!isCustomPropertyName(descriptor.name))
Expand All @@ -56,29 +58,34 @@ ExceptionOr<void> DOMCSSRegisterCustomProperty::registerProperty(Document& docum
return Exception { SyntaxError, "An initial value is mandatory except for the '*' syntax."_s };

RefPtr<CSSCustomPropertyValue> initialValue;
RefPtr<CSSVariableData> initialValueTokensForViewportUnits;

if (!descriptor.initialValue.isNull()) {
CSSTokenizer tokenizer(descriptor.initialValue);

auto dependencies = CSSPropertyParser::collectParsedCustomPropertyValueDependencies(*syntax, tokenizer.tokenRange(), strictCSSParserContext());

if (!dependencies.isEmpty())
return Exception { SyntaxError, "The given initial value must be computationally independent."_s };
auto parsedInitialValue = CustomPropertyRegistry::parseInitialValue(document, descriptor.name, *syntax, tokenizer.tokenRange());

// We don't need to provide a real context style since only computationally independent values are allowed (no 'em' etc).
auto placeholderStyle = RenderStyle::create();
Style::Builder builder { placeholderStyle, { document, RenderStyle::defaultStyle() }, { }, { } };
if (!parsedInitialValue) {
if (parsedInitialValue.error() == CustomPropertyRegistry::ParseInitialValueError::NotComputationallyIndependent)
return Exception { SyntaxError, "The given initial value must be computationally independent."_s };

initialValue = CSSPropertyParser::parseTypedCustomPropertyInitialValue(descriptor.name, *syntax, tokenizer.tokenRange(), builder.state(), { document });

if (!initialValue)
ASSERT(parsedInitialValue.error() == CustomPropertyRegistry::ParseInitialValueError::DidNotParse);
return Exception { SyntaxError, "The given initial value does not parse for the given syntax."_s };
}

initialValue = parsedInitialValue->first;
if (parsedInitialValue->second == CustomPropertyRegistry::ViewportUnitDependency::Yes) {
initialValueTokensForViewportUnits = CSSVariableData::create(tokenizer.tokenRange());
document.setHasStyleWithViewportUnits();
}
}

auto property = CSSRegisteredCustomProperty {
descriptor.name,
*syntax,
descriptor.inherits,
WTFMove(initialValue)
WTFMove(initialValue),
WTFMove(initialValueTokensForViewportUnits)
};

auto& registry = document.styleScope().customPropertyRegistry();
Expand Down
93 changes: 68 additions & 25 deletions Source/WebCore/style/CustomPropertyRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ bool CustomPropertyRegistry::registerFromAPI(CSSRegisteredCustomProperty&& prope
// First registration wins.
// https://drafts.css-houdini.org/css-properties-values-api/#determining-registration
auto success = m_propertiesFromAPI.ensure(property.name, [&] {
return makeUnique<const CSSRegisteredCustomProperty>(WTFMove(property));
return makeUniqueRef<CSSRegisteredCustomProperty>(WTFMove(property));
}).isNewEntry;

if (success) {
Expand All @@ -96,44 +96,40 @@ void CustomPropertyRegistry::registerFromStylesheet(const StyleRuleProperty::Des

auto& document = m_scope.document();

auto initialValue = [&]() -> RefPtr<CSSCustomPropertyValue> {
if (!descriptor.initialValue) {
// "If the value of the syntax descriptor is the universal syntax definition, then the initial-value descriptor is optional.
// If omitted, the initial value of the property is the guaranteed-invalid value."
// https://drafts.css-houdini.org/css-properties-values-api/#initial-value-descriptor
if (syntax->isUniversal())
return CSSCustomPropertyValue::createWithID(descriptor.name, CSSValueInvalid);

return nullptr;
}
RefPtr<CSSCustomPropertyValue> initialValue;
RefPtr<CSSVariableData> initialValueTokensForViewportUnits;

if (descriptor.initialValue) {
auto tokenRange = descriptor.initialValue->tokenRange();

// FIXME: This parses twice.
auto dependencies = CSSPropertyParser::collectParsedCustomPropertyValueDependencies(*syntax, tokenRange, strictCSSParserContext());
if (!dependencies.isEmpty())
return nullptr;

// We don't need to provide a real context style since only computationally independent values are allowed (no 'em' etc).
auto placeholderStyle = RenderStyle::create();
Style::Builder builder { placeholderStyle, { document, RenderStyle::defaultStyle() }, { }, { } };
auto parsedInitialValue = parseInitialValue(document, descriptor.name, *syntax, tokenRange);
if (!parsedInitialValue)
return;

return CSSPropertyParser::parseTypedCustomPropertyInitialValue(descriptor.name, *syntax, tokenRange, builder.state(), { document });
}();

if (!initialValue)
initialValue = parsedInitialValue->first;
if (parsedInitialValue->second == ViewportUnitDependency::Yes) {
initialValueTokensForViewportUnits = CSSVariableData::create(tokenRange);
m_scope.document().setHasStyleWithViewportUnits();
}
} else if (syntax->isUniversal()) {
// "If the value of the syntax descriptor is the universal syntax definition, then the initial-value descriptor is optional.
// If omitted, the initial value of the property is the guaranteed-invalid value."
// https://drafts.css-houdini.org/css-properties-values-api/#initial-value-descriptor
initialValue = CSSCustomPropertyValue::createWithID(descriptor.name, CSSValueInvalid);
} else
return;

auto property = CSSRegisteredCustomProperty {
AtomString { descriptor.name },
*syntax,
*descriptor.inherits,
WTFMove(initialValue)
WTFMove(initialValue),
WTFMove(initialValueTokensForViewportUnits)
};

// Last rule wins.
// https://drafts.css-houdini.org/css-properties-values-api/#determining-registration
m_propertiesFromStylesheet.set(property.name, makeUnique<const CSSRegisteredCustomProperty>(WTFMove(property)));
m_propertiesFromStylesheet.set(property.name, makeUniqueRef<CSSRegisteredCustomProperty>(WTFMove(property)));

invalidate(property.name);
}
Expand Down Expand Up @@ -194,5 +190,52 @@ void CustomPropertyRegistry::notifyAnimationsOfCustomPropertyRegistration(const
}
}

auto CustomPropertyRegistry::parseInitialValue(const Document& document, const AtomString& propertyName, const CSSCustomPropertySyntax& syntax, CSSParserTokenRange tokenRange) -> Expected<std::pair<RefPtr<CSSCustomPropertyValue>, ViewportUnitDependency>, ParseInitialValueError>
{
// FIXME: This parses twice.
auto dependencies = CSSPropertyParser::collectParsedCustomPropertyValueDependencies(syntax, tokenRange, strictCSSParserContext());
if (!dependencies.isComputationallyIndependent())
return makeUnexpected(ParseInitialValueError::NotComputationallyIndependent);

// We don't need to provide a real context style since only computationally independent values are allowed (no 'em' etc).
auto placeholderStyle = RenderStyle::create();
Style::Builder builder { placeholderStyle, { document, RenderStyle::defaultStyle() }, { }, { } };

auto initialValue = CSSPropertyParser::parseTypedCustomPropertyInitialValue(propertyName, syntax, tokenRange, builder.state(), { document });
if (!initialValue)
return makeUnexpected(ParseInitialValueError::DidNotParse);

return std::pair { WTFMove(initialValue), dependencies.viewportDimensions ? ViewportUnitDependency::Yes : ViewportUnitDependency::No };
}

bool CustomPropertyRegistry::invalidatePropertiesWithViewportUnits(Document& document)
{
bool invalidatedAny = false;

auto invalidatePropertiesWithViewportUnits = [&](auto& map) {
for (auto& property : map.values()) {
if (!property->initialValueTokensForViewportUnits)
continue;

auto tokenRange = property->initialValueTokensForViewportUnits->tokenRange();
auto parsedInitialValue = parseInitialValue(document, property->name, property->syntax, tokenRange);
if (!parsedInitialValue) {
ASSERT_NOT_REACHED();
continue;
}
property->initialValue = WTFMove(parsedInitialValue->first);
ASSERT(parsedInitialValue->second == ViewportUnitDependency::Yes);

invalidate(property->name);
invalidatedAny = true;
}
};

invalidatePropertiesWithViewportUnits(m_propertiesFromAPI);
invalidatePropertiesWithViewportUnits(m_propertiesFromStylesheet);

return invalidatedAny;
}

}
}
10 changes: 8 additions & 2 deletions Source/WebCore/style/CustomPropertyRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,20 @@ class CustomPropertyRegistry {

const RenderStyle& initialValuePrototypeStyle() const;

bool invalidatePropertiesWithViewportUnits(Document&);

enum class ViewportUnitDependency : bool { No, Yes };
enum class ParseInitialValueError : uint8_t { NotComputationallyIndependent, DidNotParse };
static Expected<std::pair<RefPtr<CSSCustomPropertyValue>, ViewportUnitDependency>, ParseInitialValueError> parseInitialValue(const Document&, const AtomString& propertyName, const CSSCustomPropertySyntax&, CSSParserTokenRange);

private:
void invalidate(const AtomString&);
void notifyAnimationsOfCustomPropertyRegistration(const AtomString&);

Scope& m_scope;

HashMap<AtomString, std::unique_ptr<const CSSRegisteredCustomProperty>> m_propertiesFromAPI;
HashMap<AtomString, std::unique_ptr<const CSSRegisteredCustomProperty>> m_propertiesFromStylesheet;
HashMap<AtomString, UniqueRef<CSSRegisteredCustomProperty>> m_propertiesFromAPI;
HashMap<AtomString, UniqueRef<CSSRegisteredCustomProperty>> m_propertiesFromStylesheet;

mutable std::unique_ptr<RenderStyle> m_initialValuePrototypeStyle;
mutable bool m_hasInvalidPrototypeStyle { false };
Expand Down
8 changes: 8 additions & 0 deletions Source/WebCore/style/StyleScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,14 @@ void Scope::didChangeViewportSize()
return;
resolver->clearCachedDeclarationsAffectedByViewportUnits();

if (customPropertyRegistry().invalidatePropertiesWithViewportUnits(m_document)) {
if (!m_shadowRoot) {
if (auto element = m_document.documentElement())
element->invalidateStyleForSubtree();
}
return;
}

// 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();
Expand Down

0 comments on commit bb27e4a

Please sign in to comment.