From d9638f22c638e18141102216ebd265028d6aef07 Mon Sep 17 00:00:00 2001 From: Xiaocheng Hu Date: Wed, 25 Jan 2023 01:45:09 +0000 Subject: [PATCH] [anchor-position] Implement anchor functions as tree-scoped CSS values This patch is a follow up and also the major motivation of the previous patch crrev.com/c/4167268. It not only rewrites the existing support without ScopedCSSValue, but also enables CSS transitions with such values, and in particular, transitions that involve more than onre tree scopes, which is impossible to support with ScopedCSSValue. Bug: 1395026 Change-Id: Id5d99024384e0b9d6ab438ae04c3bb36e18b83d9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4185495 Reviewed-by: Anders Hartvoll Ruud Commit-Queue: Xiaocheng Hu Cr-Commit-Position: refs/heads/main@{#1096559} --- .../renderer/core/css/css_length_resolver.h | 6 -- .../core/css/css_math_expression_node.cc | 61 +++++++++++-- .../core/css/css_math_expression_node.h | 30 ++++++- .../core/css/css_math_function_value.cc | 11 ++- .../core/css/css_math_function_value.h | 2 + .../renderer/core/css/css_properties.json5 | 10 --- .../core/css/css_to_length_conversion_data.h | 14 --- .../blink/renderer/core/css/css_value.cc | 2 + .../css/resolver/style_builder_converter.cc | 38 +------- .../css/resolver/style_builder_converter.h | 6 -- .../core/css/resolver/style_resolver_state.h | 6 -- .../anchor-transition-001.html | 78 ++++++++++++++++ .../anchor-transition-002.html | 70 +++++++++++++++ .../anchor-transition-003.html | 89 +++++++++++++++++++ 14 files changed, 333 insertions(+), 90 deletions(-) create mode 100644 third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-001.html create mode 100644 third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-002.html create mode 100644 third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-003.html diff --git a/third_party/blink/renderer/core/css/css_length_resolver.h b/third_party/blink/renderer/core/css/css_length_resolver.h index a2352f5156ec1..58388e2762b46 100644 --- a/third_party/blink/renderer/core/css/css_length_resolver.h +++ b/third_party/blink/renderer/core/css/css_length_resolver.h @@ -15,8 +15,6 @@ namespace blink { -class TreeScope; - class CORE_EXPORT CSSLengthResolver { public: explicit CSSLengthResolver(float zoom) : zoom_(zoom) {} @@ -49,10 +47,6 @@ class CORE_EXPORT CSSLengthResolver { virtual WritingMode GetWritingMode() const = 0; - // Some length functions (anchor() and anchor-size()) contain tree-scoped - // name references. Returns that tree scope. - virtual const TreeScope* GetTreeScope() const { return nullptr; } - float Zoom() const { return zoom_; } void SetZoom(float zoom) { DCHECK(std::isfinite(zoom)); diff --git a/third_party/blink/renderer/core/css/css_math_expression_node.cc b/third_party/blink/renderer/core/css/css_math_expression_node.cc index 6fe4ec338d9c6..a69dc427dcf1d 100644 --- a/third_party/blink/renderer/core/css/css_math_expression_node.cc +++ b/third_party/blink/renderer/core/css/css_math_expression_node.cc @@ -234,7 +234,8 @@ CSSMathExpressionNumericLiteral* CSSMathExpressionNumericLiteral::Create( CSSMathExpressionNumericLiteral::CSSMathExpressionNumericLiteral( const CSSNumericLiteralValue* value) : CSSMathExpressionNode(UnitCategory(value->GetType()), - false /* has_comparisons*/), + false /* has_comparisons*/, + false /* needs_tree_scope_population*/), value_(value) {} bool CSSMathExpressionNumericLiteral::IsZero() const { @@ -704,7 +705,8 @@ CSSMathExpressionOperation::CSSMathExpressionOperation( CalculationCategory category) : CSSMathExpressionNode( category, - left_side->HasComparisons() || right_side->HasComparisons()), + left_side->HasComparisons() || right_side->HasComparisons(), + !left_side->IsScopedValue() || !right_side->IsScopedValue()), operands_({left_side, right_side}), operator_(op) {} @@ -718,13 +720,24 @@ static bool AnyOperandHasComparisons( return false; } +static bool AnyOperandNeedsTreeScopePopulation( + CSSMathExpressionOperation::Operands& operands) { + for (const CSSMathExpressionNode* operand : operands) { + if (!operand->IsScopedValue()) { + return true; + } + } + return false; +} + CSSMathExpressionOperation::CSSMathExpressionOperation( CalculationCategory category, Operands&& operands, CSSMathOperator op) : CSSMathExpressionNode( category, - IsComparison(op) || AnyOperandHasComparisons(operands)), + IsComparison(op) || AnyOperandHasComparisons(operands), + AnyOperandNeedsTreeScopePopulation(operands)), operands_(std::move(operands)), operator_(op) {} @@ -1196,6 +1209,16 @@ double CSSMathExpressionOperation::EvaluateOperator( return 0; } +const CSSMathExpressionNode& CSSMathExpressionOperation::PopulateWithTreeScope( + const TreeScope* tree_scope) const { + Operands populated_operands; + for (const CSSMathExpressionNode* op : operands_) { + populated_operands.push_back(&op->EnsureScopedValue(tree_scope)); + } + return *MakeGarbageCollected( + Category(), std::move(populated_operands), operator_); +} + #if DCHECK_IS_ON() bool CSSMathExpressionOperation::InvolvesPercentageComparisons() const { if (IsMinOrMax() && Category() == kCalcPercent && operands_.size() > 1u) { @@ -1219,7 +1242,10 @@ CSSMathExpressionAnchorQuery::CSSMathExpressionAnchorQuery( const CSSCustomIdentValue* anchor_name, const CSSValue& value, const CSSPrimitiveValue* fallback) - : CSSMathExpressionNode(kCalcPercentLength, false /* has_comparisons */), + : CSSMathExpressionNode(kCalcPercentLength, + false /* has_comparisons */, + (anchor_name && !anchor_name->IsScopedValue()) || + (fallback && !fallback->IsScopedValue())), type_(type), anchor_name_(anchor_name), value_(value), @@ -1306,9 +1332,10 @@ AnchorSizeValue CSSValueIDToAnchorSizeValueEnum(CSSValueID value) { scoped_refptr CSSMathExpressionAnchorQuery::ToCalculationExpression( const CSSLengthResolver& length_resolver) const { + DCHECK(IsScopedValue()); ScopedCSSName* anchor_name = anchor_name_ ? MakeGarbageCollected( - anchor_name_->Value(), length_resolver.GetTreeScope()) + anchor_name_->Value(), anchor_name_->GetTreeScope()) : nullptr; Length fallback = fallback_ ? fallback_->ConvertToLength(length_resolver) : Length::Fixed(0); @@ -1332,6 +1359,20 @@ CSSMathExpressionAnchorQuery::ToCalculationExpression( fallback); } +const CSSMathExpressionNode& +CSSMathExpressionAnchorQuery::PopulateWithTreeScope( + const TreeScope* tree_scope) const { + return *MakeGarbageCollected( + type_, + anchor_name_ ? To( + &anchor_name_->EnsureScopedValue(tree_scope)) + : nullptr, + *value_, + fallback_ + ? To(&fallback_->EnsureScopedValue(tree_scope)) + : nullptr); +} + void CSSMathExpressionAnchorQuery::Trace(Visitor* visitor) const { visitor->Trace(anchor_name_); visitor->Trace(value_); @@ -1825,10 +1866,12 @@ CSSMathExpressionNode* CSSMathExpressionNode::Create( CSSAnchorQueryType type = anchor_query.Type() == AnchorQueryType::kAnchor ? CSSAnchorQueryType::kAnchor : CSSAnchorQueryType::kAnchorSize; - CSSCustomIdentValue* anchor_name = - anchor_query.AnchorName() ? MakeGarbageCollected( - anchor_query.AnchorName()->GetName()) - : nullptr; + const CSSCustomIdentValue* anchor_name = nullptr; + if (const ScopedCSSName* passed_name = anchor_query.AnchorName()) { + anchor_name = To( + &MakeGarbageCollected(passed_name->GetName()) + ->EnsureScopedValue(passed_name->GetTreeScope())); + } CSSValue* value = AnchorQueryValueToCSSValue(anchor_query); CSSPrimitiveValue* fallback = CSSPrimitiveValue::CreateFromLength( anchor_query.GetFallback(), /* zoom */ 1); diff --git a/third_party/blink/renderer/core/css/css_math_expression_node.h b/third_party/blink/renderer/core/css/css_math_expression_node.h index c7643d699f66f..7217304a0a7db 100644 --- a/third_party/blink/renderer/core/css/css_math_expression_node.h +++ b/third_party/blink/renderer/core/css/css_math_expression_node.h @@ -141,6 +141,17 @@ class CORE_EXPORT CSSMathExpressionNode void SetIsNestedCalc() { is_nested_calc_ = true; } bool HasComparisons() const { return has_comparisons_; } + bool IsScopedValue() const { return !needs_tree_scope_population_; } + + const CSSMathExpressionNode& EnsureScopedValue( + const TreeScope* tree_scope) const { + if (!needs_tree_scope_population_) { + return *this; + } + return PopulateWithTreeScope(tree_scope); + } + virtual const CSSMathExpressionNode& PopulateWithTreeScope( + const TreeScope*) const = 0; #if DCHECK_IS_ON() // There's a subtle issue in comparing two percentages, e.g., min(10%, 20%). @@ -153,14 +164,19 @@ class CORE_EXPORT CSSMathExpressionNode virtual void Trace(Visitor* visitor) const {} protected: - CSSMathExpressionNode(CalculationCategory category, bool has_comparisons) - : category_(category), has_comparisons_(has_comparisons) { + CSSMathExpressionNode(CalculationCategory category, + bool has_comparisons, + bool needs_tree_scope_population) + : category_(category), + has_comparisons_(has_comparisons), + needs_tree_scope_population_(needs_tree_scope_population) { DCHECK_NE(category, kCalcOther); } CalculationCategory category_; bool is_nested_calc_ = false; bool has_comparisons_; + bool needs_tree_scope_population_; }; class CORE_EXPORT CSSMathExpressionNumericLiteral final @@ -178,6 +194,12 @@ class CORE_EXPORT CSSMathExpressionNumericLiteral final bool IsNumericLiteral() const final { return true; } + const CSSMathExpressionNode& PopulateWithTreeScope( + const TreeScope* tree_scope) const final { + NOTREACHED(); + return *this; + } + bool IsZero() const final; String CustomCSSText() const final; scoped_refptr ToCalculationExpression( @@ -273,6 +295,8 @@ class CORE_EXPORT CSSMathExpressionOperation final String CustomCSSText() const final; bool operator==(const CSSMathExpressionNode& exp) const final; CSSPrimitiveValue::UnitType ResolvedUnitType() const final; + const CSSMathExpressionNode& PopulateWithTreeScope( + const TreeScope*) const final; void Trace(Visitor* visitor) const final; #if DCHECK_IS_ON() @@ -365,6 +389,8 @@ class CORE_EXPORT CSSMathExpressionAnchorQuery final scoped_refptr ToCalculationExpression( const CSSLengthResolver&) const final; bool operator==(const CSSMathExpressionNode& other) const final; + const CSSMathExpressionNode& PopulateWithTreeScope( + const TreeScope*) const final; void Trace(Visitor* visitor) const final; #if DCHECK_IS_ON() diff --git a/third_party/blink/renderer/core/css/css_math_function_value.cc b/third_party/blink/renderer/core/css/css_math_function_value.cc index 6c1930e8016d0..7d5ef4b8cc834 100644 --- a/third_party/blink/renderer/core/css/css_math_function_value.cc +++ b/third_party/blink/renderer/core/css/css_math_function_value.cc @@ -29,7 +29,9 @@ CSSMathFunctionValue::CSSMathFunctionValue( CSSPrimitiveValue::ValueRange range) : CSSPrimitiveValue(kMathFunctionClass), expression_(expression), - value_range_in_target_context_(range) {} + value_range_in_target_context_(range) { + needs_tree_scope_population_ = !expression->IsScopedValue(); +} // static CSSMathFunctionValue* CSSMathFunctionValue::Create( @@ -166,4 +168,11 @@ scoped_refptr CSSMathFunctionValue::ToCalcValue( AllowsNegativePercentageReference()); } +const CSSValue& CSSMathFunctionValue::PopulateWithTreeScope( + const TreeScope* tree_scope) const { + return *MakeGarbageCollected( + &expression_->PopulateWithTreeScope(tree_scope), + value_range_in_target_context_); +} + } // namespace blink diff --git a/third_party/blink/renderer/core/css/css_math_function_value.h b/third_party/blink/renderer/core/css/css_math_function_value.h index bf7e7625ebc12..550c090707c45 100644 --- a/third_party/blink/renderer/core/css/css_math_function_value.h +++ b/third_party/blink/renderer/core/css/css_math_function_value.h @@ -87,6 +87,8 @@ class CORE_EXPORT CSSMathFunctionValue : public CSSPrimitiveValue { bool HasComparisons() const { return expression_->HasComparisons(); } + const CSSValue& PopulateWithTreeScope(const TreeScope*) const; + void TraceAfterDispatch(blink::Visitor* visitor) const; private: diff --git a/third_party/blink/renderer/core/css/css_properties.json5 b/third_party/blink/renderer/core/css/css_properties.json5 index 79d8fbece9150..782b2a125a61e 100644 --- a/third_party/blink/renderer/core/css/css_properties.json5 +++ b/third_party/blink/renderer/core/css/css_properties.json5 @@ -2293,7 +2293,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "box-shadow", @@ -3008,7 +3007,6 @@ supports_incremental_style: true, valid_for_formatted_text: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "hyphenate-limit-chars", @@ -3132,7 +3130,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "letter-spacing", @@ -3400,7 +3397,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "max-width", @@ -3418,7 +3414,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "min-height", @@ -3435,7 +3430,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "min-width", @@ -3452,7 +3446,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "mix-blend-mode", @@ -4055,7 +4048,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "r", @@ -4977,7 +4969,6 @@ }, supports_incremental_style: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "touch-action", @@ -5901,7 +5892,6 @@ supports_incremental_style: true, valid_for_formatted_text: true, valid_for_position_fallback: true, - tree_scoped_value: true, }, { name: "will-change", diff --git a/third_party/blink/renderer/core/css/css_to_length_conversion_data.h b/third_party/blink/renderer/core/css/css_to_length_conversion_data.h index fc1fc5e363eed..977b6e341e6b2 100644 --- a/third_party/blink/renderer/core/css/css_to_length_conversion_data.h +++ b/third_party/blink/renderer/core/css/css_to_length_conversion_data.h @@ -300,20 +300,6 @@ class CORE_EXPORT CSSToLengthConversionData : public CSSLengthResolver { mutable Flags* flags_ = nullptr; }; -class ScopedCSSToLengthConversionData : public CSSToLengthConversionData { - STACK_ALLOCATED(); - - public: - ScopedCSSToLengthConversionData(const CSSToLengthConversionData& data, - const TreeScope* scope) - : CSSToLengthConversionData(data), tree_scope_(scope) {} - - const TreeScope* GetTreeScope() const override { return tree_scope_; } - - private: - const TreeScope* tree_scope_; -}; - } // namespace blink #endif // THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_TO_LENGTH_CONVERSION_DATA_H_ diff --git a/third_party/blink/renderer/core/css/css_value.cc b/third_party/blink/renderer/core/css/css_value.cc index da7d54817f789..92e7c1ca620f0 100644 --- a/third_party/blink/renderer/core/css/css_value.cc +++ b/third_party/blink/renderer/core/css/css_value.cc @@ -464,6 +464,8 @@ const CSSValue& CSSValue::PopulateWithTreeScope( switch (GetClassType()) { case kCustomIdentClass: return To(this)->PopulateWithTreeScope(tree_scope); + case kMathFunctionClass: + return To(this)->PopulateWithTreeScope(tree_scope); default: NOTREACHED(); return *this; diff --git a/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc b/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc index f3c1d3b715bed..ca80b65f5368b 100644 --- a/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc +++ b/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc @@ -1514,13 +1514,6 @@ Length StyleBuilderConverter::ConvertLength(const StyleResolverState& state, return To(value).ConvertToLength( state.CssToLengthConversionData()); } -Length StyleBuilderConverter::ConvertLength( - const StyleResolverState& state, - const ScopedCSSValue& scoped_value) { - return To(scoped_value.GetCSSValue()) - .ConvertToLength(state.GetScopedCSSToLengthConversionData( - scoped_value.GetTreeScope())); -} UnzoomedLength StyleBuilderConverter::ConvertUnzoomedLength( StyleResolverState& state, @@ -1555,35 +1548,19 @@ float StyleBuilderConverter::ConvertZoom(const StyleResolverState& state, Length StyleBuilderConverter::ConvertLengthOrAuto( const StyleResolverState& state, const CSSValue& value) { - return ConvertLengthOrAuto(state, - ScopedCSSValue(value, nullptr /* TreeScope */)); -} - -Length StyleBuilderConverter::ConvertLengthOrAuto( - const StyleResolverState& state, - const ScopedCSSValue& scoped_value) { - const CSSValue& value = scoped_value.GetCSSValue(); auto* identifier_value = DynamicTo(value); if (identifier_value && identifier_value->GetValueID() == CSSValueID::kAuto) { return Length::Auto(); } return To(value).ConvertToLength( - state.GetScopedCSSToLengthConversionData(scoped_value.GetTreeScope())); + state.CssToLengthConversionData()); } Length StyleBuilderConverter::ConvertLengthSizing(StyleResolverState& state, const CSSValue& value) { - return ConvertLengthSizing(state, - ScopedCSSValue(value, nullptr /* TreeScope */)); -} - -Length StyleBuilderConverter::ConvertLengthSizing( - StyleResolverState& state, - const ScopedCSSValue& scoped_value) { - const CSSValue& value = scoped_value.GetCSSValue(); const auto* identifier_value = DynamicTo(value); if (!identifier_value) { - return ConvertLength(state, scoped_value); + return ConvertLength(state, value); } switch (identifier_value->GetValueID()) { @@ -1617,17 +1594,6 @@ Length StyleBuilderConverter::ConvertLengthMaxSizing(StyleResolverState& state, return ConvertLengthSizing(state, value); } -Length StyleBuilderConverter::ConvertLengthMaxSizing( - StyleResolverState& state, - const ScopedCSSValue& scoped_value) { - const CSSValue& value = scoped_value.GetCSSValue(); - auto* identifier_value = DynamicTo(value); - if (identifier_value && identifier_value->GetValueID() == CSSValueID::kNone) { - return Length::None(); - } - return ConvertLengthSizing(state, scoped_value); -} - TabSize StyleBuilderConverter::ConvertLengthOrTabSpaces( StyleResolverState& state, const CSSValue& value) { diff --git a/third_party/blink/renderer/core/css/resolver/style_builder_converter.h b/third_party/blink/renderer/core/css/resolver/style_builder_converter.h index 75637a8e055b1..e480d2f8396aa 100644 --- a/third_party/blink/renderer/core/css/resolver/style_builder_converter.h +++ b/third_party/blink/renderer/core/css/resolver/style_builder_converter.h @@ -193,20 +193,14 @@ class StyleBuilderConverter { static absl::optional ConvertGapLength(const StyleResolverState&, const CSSValue&); static Length ConvertLength(const StyleResolverState&, const CSSValue&); - static Length ConvertLength(const StyleResolverState&, const ScopedCSSValue&); static UnzoomedLength ConvertUnzoomedLength(StyleResolverState&, const CSSValue&); static float ConvertZoom(const StyleResolverState&, const CSSValue&); static TimelineInset ConvertSingleTimelineInset(StyleResolverState&, const CSSValue&); static Length ConvertLengthOrAuto(const StyleResolverState&, const CSSValue&); - static Length ConvertLengthOrAuto(const StyleResolverState&, - const ScopedCSSValue&); static Length ConvertLengthSizing(StyleResolverState&, const CSSValue&); - static Length ConvertLengthSizing(StyleResolverState&, const ScopedCSSValue&); static Length ConvertLengthMaxSizing(StyleResolverState&, const CSSValue&); - static Length ConvertLengthMaxSizing(StyleResolverState&, - const ScopedCSSValue&); static TabSize ConvertLengthOrTabSpaces(StyleResolverState&, const CSSValue&); static Length ConvertLineHeight(StyleResolverState&, const CSSValue&); static float ConvertNumberOrPercentage(StyleResolverState&, const CSSValue&); diff --git a/third_party/blink/renderer/core/css/resolver/style_resolver_state.h b/third_party/blink/renderer/core/css/resolver/style_resolver_state.h index d9a70b96ab4bc..c78b32ef045f3 100644 --- a/third_party/blink/renderer/core/css/resolver/style_resolver_state.h +++ b/third_party/blink/renderer/core/css/resolver/style_resolver_state.h @@ -100,12 +100,6 @@ class CORE_EXPORT StyleResolverState { CSSToLengthConversionData FontSizeConversionData(); CSSToLengthConversionData UnzoomedLengthConversionData(); - ScopedCSSToLengthConversionData GetScopedCSSToLengthConversionData( - const TreeScope* scope) const { - return ScopedCSSToLengthConversionData(css_to_length_conversion_data_, - scope); - } - CSSToLengthConversionData::Flags TakeLengthConversionFlags() { CSSToLengthConversionData::Flags flags = length_conversion_flags_; length_conversion_flags_ = 0; diff --git a/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-001.html b/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-001.html new file mode 100644 index 0000000000000..b5849510e838b --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-001.html @@ -0,0 +1,78 @@ + +Tests CSS transition of anchor() and anchor-size() functions + + + + + + + + +
+
+
+ + + diff --git a/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-002.html b/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-002.html new file mode 100644 index 0000000000000..b6a1ff4511ac9 --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-002.html @@ -0,0 +1,70 @@ + +Tests CSS transition of anchor() across tree scopes + + + + + + + + +
+
+ + diff --git a/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-003.html b/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-003.html new file mode 100644 index 0000000000000..e744127514117 --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-transition-003.html @@ -0,0 +1,89 @@ + +Tests CSS transition of anchor() across three tree scopes + + + + + + + + +
+
+ +