From b6b27b64ed49dbb40c15bcde8796a6c93cc9bd42 Mon Sep 17 00:00:00 2001 From: Anders Hartvoll Ruud Date: Fri, 8 Mar 2024 19:07:13 +0000 Subject: [PATCH] [anchor] Always resolve anchor()/anchor-size() computed-value time This CL removes the option of evaluating anchor() and anchor-size() functions at used-value time, per recent CSSWG resolution [1]. This eliminates the need for an AnchorEvaluator to resolve a Length, which means don't need to send that around to the extent we did before. It's still needed in some cases, because AnchorEvaluatorImpl is used for some secondary purposes in absolute_utils.cc. We still use CalculationExpressionAnchorQueryNode to represent the anchor query during Length::AnchorEvaluator::Evaluate for now. A follow-up CL will address this. Except for the removal of the feature flag, there should be no behavior change. Bug: 41483417 [1] https://github.com/w3c/csswg-drafts/issues/9598 Change-Id: I068a57fcb6b5c90e133a36e095e8bbf25861714d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5355673 Reviewed-by: Ian Kilpatrick Commit-Queue: Anders Hartvoll Ruud Cr-Commit-Position: refs/heads/main@{#1270318} --- ...alculation_expression_anchor_query_node.cc | 15 +- .../renderer/core/css/css_length_resolver.h | 3 +- .../core/css/css_math_expression_node.cc | 16 +- .../css/css_to_length_conversion_data_test.cc | 9 - .../cssom/computed_style_property_map_test.cc | 13 -- .../core/css/properties/css_property_test.cc | 10 - .../core/css/resolver/style_resolver.cc | 3 +- .../core/css/resolver/style_resolver_test.cc | 172 ------------------ .../renderer/core/css/style_engine_test.cc | 8 - .../renderer/core/layout/absolute_utils.cc | 50 ++--- .../renderer/core/layout/absolute_utils.h | 2 - .../core/layout/absolute_utils_test.cc | 6 +- .../renderer/core/layout/length_utils.cc | 110 +++++------ .../blink/renderer/core/layout/length_utils.h | 51 ++---- .../core/layout/out_of_flow_layout_part.cc | 20 +- .../blink/renderer/platform/geometry/length.h | 1 - .../platform/runtime_enabled_features.json5 | 6 - 17 files changed, 100 insertions(+), 395 deletions(-) diff --git a/third_party/blink/renderer/core/css/calculation_expression_anchor_query_node.cc b/third_party/blink/renderer/core/css/calculation_expression_anchor_query_node.cc index 3ebb1969a2ee3..b6d91dae285d9 100644 --- a/third_party/blink/renderer/core/css/calculation_expression_anchor_query_node.cc +++ b/third_party/blink/renderer/core/css/calculation_expression_anchor_query_node.cc @@ -87,18 +87,9 @@ CalculationExpressionAnchorQueryNode::Zoom(double factor) const { float CalculationExpressionAnchorQueryNode::Evaluate( float max_value, - const Length::EvaluationInput& input) const { - if (input.anchor_evaluator) { - if (const std::optional value = - input.anchor_evaluator->Evaluate(*this)) { - return value->ToFloat(); - } - } - // If we did not provide an AnchorEvaluator, then this is not - // for an absolutely-positioned element, and we should use the fallback. - // - // https://drafts.csswg.org/css-anchor-position-1/#valid-anchor-function - return FloatValueForLength(fallback_, max_value, input); + const Length::EvaluationInput&) const { + // TODO(crbug.com//41483417): Remove CalculationExpressionAnchorQueryNode. + return 0; } } // namespace blink 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 0c397f3d730fe..124f77531478b 100644 --- a/third_party/blink/renderer/core/css/css_length_resolver.h +++ b/third_party/blink/renderer/core/css/css_length_resolver.h @@ -61,8 +61,7 @@ class CORE_EXPORT CSSLengthResolver { // https://drafts.csswg.org/css-anchor-position-1/ virtual void ReferenceAnchor() const = 0; - // The AnchorEvaluator used to evaluate anchor()/anchor-size() queries, - // when the runtime flag CSSAnchorPositioningComputeAnchor is enabled. + // The AnchorEvaluator used to evaluate anchor()/anchor-size() queries. virtual Length::AnchorEvaluator* AnchorEvaluator() const { return nullptr; } float Zoom() const { return 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 886e36fbfdd3c..7625f0f23afc9 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 @@ -278,9 +278,7 @@ bool CanEagerlySimplify(const CSSMathExpressionNode* operand) { return true; case CalculationResultCategory::kCalcLength: return !CSSPrimitiveValue::IsRelativeUnit(operand->ResolvedUnitType()) && - (!RuntimeEnabledFeatures:: - CSSAnchorPositioningComputeAnchorEnabled() || - !operand->IsAnchorQuery()); + !operand->IsAnchorQuery(); default: return false; } @@ -2352,10 +2350,6 @@ namespace { CalculationResultCategory AnchorQueryCategory( const CSSPrimitiveValue* fallback) { - if (!RuntimeEnabledFeatures::CSSAnchorPositioningComputeAnchorEnabled()) { - // Anchor queries are resolved used-value time. - return kCalcLengthFunction; - } // Note that the main (non-fallback) result of an anchor query is always // a kCalcLength, so the only thing that can make our overall result anything // else is the fallback. @@ -2405,8 +2399,6 @@ double CSSMathExpressionAnchorQuery::ComputeDouble( // AnchorQueryCategory), in which case we'll reach ToCalculationExpression // instead. - CHECK(RuntimeEnabledFeatures::CSSAnchorPositioningComputeAnchorEnabled()); - // TODO(crbug.com/41483417): Remove CalculationExpressionAnchorQueryNode. scoped_refptr query = ToCalculationExpressionQuery(length_resolver); @@ -2503,11 +2495,6 @@ CSSMathExpressionAnchorQuery::ToCalculationExpression( scoped_refptr query = ToCalculationExpressionQuery(length_resolver); - // TODO(crbug.com/41483417): Remove CalculationExpressionAnchorQueryNode. - if (!RuntimeEnabledFeatures::CSSAnchorPositioningComputeAnchorEnabled()) { - return query; - } - Length result; if (std::optional px = EvaluateQuery(*query, length_resolver)) { @@ -2535,6 +2522,7 @@ std::optional CSSMathExpressionAnchorQuery::EvaluateQuery( scoped_refptr CSSMathExpressionAnchorQuery::ToCalculationExpressionQuery( const CSSLengthResolver& length_resolver) const { + // TODO(crbug.com/41483417): Remove CalculationExpressionAnchorQueryNode. DCHECK(IsScopedValue()); AnchorSpecifierValue* anchor_specifier = AnchorSpecifierValue::Default(); if (const auto* implicit = diff --git a/third_party/blink/renderer/core/css/css_to_length_conversion_data_test.cc b/third_party/blink/renderer/core/css/css_to_length_conversion_data_test.cc index 53e5159d8265d..38a620dbfe359 100644 --- a/third_party/blink/renderer/core/css/css_to_length_conversion_data_test.cc +++ b/third_party/blink/renderer/core/css/css_to_length_conversion_data_test.cc @@ -356,7 +356,6 @@ TEST_F(CSSToLengthConversionDataTest, ConversionWithoutPrimaryFont) { TEST_F(CSSToLengthConversionDataTest, AnchorFunction) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ LayoutUnit(60.0)); CSSToLengthConversionData data = @@ -371,7 +370,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorFunction) { TEST_F(CSSToLengthConversionDataTest, AnchorFunctionFallback) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ std::nullopt); CSSToLengthConversionData data = @@ -389,7 +387,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorFunctionFallback) { TEST_F(CSSToLengthConversionDataTest, AnchorSizeFunction) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ LayoutUnit(60.0)); CSSToLengthConversionData data = @@ -405,7 +402,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorSizeFunction) { TEST_F(CSSToLengthConversionDataTest, AnchorSizeFunctionFallback) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ std::nullopt); CSSToLengthConversionData data = @@ -425,7 +421,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorSizeFunctionFallback) { TEST_F(CSSToLengthConversionDataTest, AnchorWithinOtherFunction) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ std::nullopt); CSSToLengthConversionData data = @@ -458,7 +453,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorWithinOtherFunction) { TEST_F(CSSToLengthConversionDataTest, AnchorFunctionPercentageFallback) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ std::nullopt); CSSToLengthConversionData data = @@ -485,7 +479,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorFunctionPercentageFallback) { TEST_F(CSSToLengthConversionDataTest, AnchorFunctionPercentageFallbackNotTaken) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ LayoutUnit(60.0)); CSSToLengthConversionData data = @@ -503,7 +496,6 @@ TEST_F(CSSToLengthConversionDataTest, TEST_F(CSSToLengthConversionDataTest, AnchorFunctionFallbackNullEvaluator) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); CSSToLengthConversionData data = ConversionData({.anchor_evaluator = nullptr}); @@ -515,7 +507,6 @@ TEST_F(CSSToLengthConversionDataTest, AnchorFunctionFallbackNullEvaluator) { TEST_F(CSSToLengthConversionDataTest, AnchorFunctionLengthPercentageFallback) { ScopedCSSAnchorPositioningForTest anchor_feature(true); - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); TestAnchorEvaluator anchor_evaluator(/* result */ std::nullopt); CSSToLengthConversionData data = diff --git a/third_party/blink/renderer/core/css/cssom/computed_style_property_map_test.cc b/third_party/blink/renderer/core/css/cssom/computed_style_property_map_test.cc index a85f9e2b5ea3c..362d940a107f8 100644 --- a/third_party/blink/renderer/core/css/cssom/computed_style_property_map_test.cc +++ b/third_party/blink/renderer/core/css/cssom/computed_style_property_map_test.cc @@ -54,20 +54,7 @@ TEST_F(ComputedStylePropertyMapTest, TransformPerspectiveZoom) { EXPECT_EQ("perspective(100px)", style_value->toString()); } -TEST_F(ComputedStylePropertyMapTest, TopWithAnchor) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(false); - - ComputedStylePropertyMap* map = - SetBodyStyle("position: absolute; top: anchor(bottom, 17px);"); - CSSStyleValue* style_value = - map->get(GetDocument().GetExecutionContext(), "top", ASSERT_NO_EXCEPTION); - ASSERT_TRUE(style_value); - EXPECT_EQ("anchor(bottom, 17px)", style_value->toString()); -} - TEST_F(ComputedStylePropertyMapTest, TopWithAnchorComputed) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); - ComputedStylePropertyMap* map = SetBodyStyle("position: absolute; top: anchor(bottom, 17px);"); CSSStyleValue* style_value = diff --git a/third_party/blink/renderer/core/css/properties/css_property_test.cc b/third_party/blink/renderer/core/css/properties/css_property_test.cc index 6358e86796ed9..dd7fd4dfdc8ab 100644 --- a/third_party/blink/renderer/core/css/properties/css_property_test.cc +++ b/third_party/blink/renderer/core/css/properties/css_property_test.cc @@ -336,8 +336,6 @@ TEST_F(CSSPropertyTest, AlternativePropertyCycle) { } TEST_F(CSSPropertyTest, AnchorModeTop) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); - ModeCheckingAnchorEvaluator anchor_evaluator(Length::AnchorScope::Mode::kTop); StyleRecalcContext context = {.anchor_evaluator = &anchor_evaluator}; @@ -354,8 +352,6 @@ TEST_F(CSSPropertyTest, AnchorModeTop) { } TEST_F(CSSPropertyTest, AnchorModeRight) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); - ModeCheckingAnchorEvaluator anchor_evaluator( Length::AnchorScope::Mode::kRight); StyleRecalcContext context = {.anchor_evaluator = &anchor_evaluator}; @@ -373,8 +369,6 @@ TEST_F(CSSPropertyTest, AnchorModeRight) { } TEST_F(CSSPropertyTest, AnchorModeBottom) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); - ModeCheckingAnchorEvaluator anchor_evaluator( Length::AnchorScope::Mode::kBottom); StyleRecalcContext context = {.anchor_evaluator = &anchor_evaluator}; @@ -392,8 +386,6 @@ TEST_F(CSSPropertyTest, AnchorModeBottom) { } TEST_F(CSSPropertyTest, AnchorModeLeft) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); - ModeCheckingAnchorEvaluator anchor_evaluator( Length::AnchorScope::Mode::kLeft); StyleRecalcContext context = {.anchor_evaluator = &anchor_evaluator}; @@ -411,8 +403,6 @@ TEST_F(CSSPropertyTest, AnchorModeLeft) { } TEST_F(CSSPropertyTest, AnchorModeSize) { - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(true); - ModeCheckingAnchorEvaluator anchor_evaluator( Length::AnchorScope::Mode::kSize); StyleRecalcContext context = {.anchor_evaluator = &anchor_evaluator}; diff --git a/third_party/blink/renderer/core/css/resolver/style_resolver.cc b/third_party/blink/renderer/core/css/resolver/style_resolver.cc index 57b32ad35e440..c15a4aedf9802 100644 --- a/third_party/blink/renderer/core/css/resolver/style_resolver.cc +++ b/third_party/blink/renderer/core/css/resolver/style_resolver.cc @@ -2388,8 +2388,7 @@ bool StyleResolver::CanReuseBaseComputedStyle(const StyleResolverState& state) { return false; } - if (RuntimeEnabledFeatures::CSSAnchorPositioningComputeAnchorEnabled() && - base_style->HasAnchorFunctions()) { + if (base_style->HasAnchorFunctions()) { // TODO(crbug.com/41483417): Enable this optimization for styles with // anchor queries. return false; diff --git a/third_party/blink/renderer/core/css/resolver/style_resolver_test.cc b/third_party/blink/renderer/core/css/resolver/style_resolver_test.cc index c7db346deb1a5..d4cd044d85b8b 100644 --- a/third_party/blink/renderer/core/css/resolver/style_resolver_test.cc +++ b/third_party/blink/renderer/core/css/resolver/style_resolver_test.cc @@ -1667,8 +1667,6 @@ TEST_P(StyleResolverTestCQ, DependsOnStyleContainerQueries) { } TEST_P(ParameterizedStyleResolverTest, AnchorQueriesMPC) { - ScopedCSSAnchorPositioningComputeAnchorForTest scoped_feature(true); - GetDocument().documentElement()->setInnerHTML(R"HTML( -
-
- -
- - -
-
- -
- )HTML"); - - UpdateAllLifecyclePhasesForTest(); - - { - Element* left = GetElementById("left"); - Element* bottom = GetElementById("bottom"); - ShadowRoot* shadow = bottom->GetShadowRoot(); - Element* top = shadow->getElementById(AtomicString("top")); - Element* right = shadow->getElementById(AtomicString("right")); - - EXPECT_EQ(&GetDocument(), - GetAnchorQueryTreeScope(GetLeft(left->ComputedStyleRef()))); - EXPECT_EQ(&GetDocument(), - GetAnchorQueryTreeScope(GetRight(right->ComputedStyleRef()))); - EXPECT_EQ(shadow, GetAnchorQueryTreeScope(GetTop(top->ComputedStyleRef()))); - EXPECT_EQ(shadow, - GetAnchorQueryTreeScope(GetBottom(bottom->ComputedStyleRef()))); - } - - { - // Verify that it also works for logical properties. - Element* inline_start = GetElementById("inline-start"); - Element* block_end = GetElementById("block-end"); - ShadowRoot* shadow = block_end->GetShadowRoot(); - Element* block_start = shadow->getElementById(AtomicString("block-start")); - Element* inline_end = shadow->getElementById(AtomicString("inline-end")); - - EXPECT_EQ(&GetDocument(), GetAnchorQueryTreeScope( - GetLeft(inline_start->ComputedStyleRef()))); - EXPECT_EQ(&GetDocument(), GetAnchorQueryTreeScope( - GetRight(inline_end->ComputedStyleRef()))); - EXPECT_EQ(shadow, - GetAnchorQueryTreeScope(GetTop(block_start->ComputedStyleRef()))); - EXPECT_EQ(shadow, GetAnchorQueryTreeScope( - GetBottom(block_end->ComputedStyleRef()))); - } -} - -TEST_P(ParameterizedStyleResolverTest, ScopedAnchorSizeFunction) { - // See comment in test ScopedAnchorFunction. - ScopedCSSAnchorPositioningComputeAnchorForTest compute_anchor_feature(false); - - GetDocument().documentElement()->setHTMLUnsafe(R"HTML( - -
- -
- - -
-
- -
- )HTML"); - - UpdateAllLifecyclePhasesForTest(); - - Element* width = GetElementById("width"); - Element* min_width = GetElementById("min-width"); - Element* max_width = GetElementById("max-width"); - ShadowRoot* shadow1 = width->GetShadowRoot(); - ShadowRoot* shadow2 = max_width->GetShadowRoot(); - Element* height = shadow1->getElementById(AtomicString("height")); - Element* min_height = shadow2->getElementById(AtomicString("min-height")); - Element* max_height = shadow2->getElementById(AtomicString("max-height")); - - EXPECT_EQ(&GetDocument(), - GetAnchorQueryTreeScope(GetWidth(width->ComputedStyleRef()))); - EXPECT_EQ(shadow1, - GetAnchorQueryTreeScope(GetHeight(height->ComputedStyleRef()))); - EXPECT_EQ(&GetDocument(), GetAnchorQueryTreeScope( - GetMinWidth(min_width->ComputedStyleRef()))); - EXPECT_EQ(shadow2, GetAnchorQueryTreeScope( - GetMaxWidth(max_width->ComputedStyleRef()))); - EXPECT_EQ(shadow2, GetAnchorQueryTreeScope( - GetMinHeight(min_height->ComputedStyleRef()))); - EXPECT_EQ(&GetDocument(), GetAnchorQueryTreeScope( - GetMaxHeight(max_height->ComputedStyleRef()))); -} - TEST_P(ParameterizedStyleResolverTest, NoAnchorFunction) { - ScopedCSSAnchorPositioningComputeAnchorForTest scoped_feature(true); - GetDocument().documentElement()->setInnerHTML(R"HTML(