[StyleBuilder] Remove ComputedStyleBuilder::SetStyle

This API was used to directly set the style on the
ComputedStyleBuilder. This isn't needed anymore, and can directly
initialize the builder now.

There should be no behaviour change.

Bug: 1377295
Change-Id: Ia9e1cb0361b312af767e897657a549f9ee559e0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4197310
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1102391}

third_party/blink/renderer/core/animation/animation_test_helpers.cc

@@ -88,10 +88,11 @@ void EnsureInterpolatedValueCached(ActiveInterpolations* interpolations,
   // document.GetStyleResolver().ResolveStyle(element). However that would
   // require our callers to properly register every animation they pass in
   // here, which the current tests do not do.
-  auto style = document.GetStyleResolver().CreateComputedStyle();
+  const ComputedStyle& initial_style =
+      document.GetStyleResolver().InitialStyle();
   StyleResolverState state(document, *element, nullptr /* StyleRecalcContext */,
-                           StyleRequest(style.get()));
-  state.SetStyle(style);
+                           StyleRequest(&initial_style));
+  state.SetStyle(initial_style);
 
   ActiveInterpolationsMap map;
   map.Set(PropertyHandle("--unused"), interpolations);

third_party/blink/renderer/core/animation/transition_keyframe.cc

@@ -45,7 +45,7 @@ void TransitionKeyframe::AddKeyframePropertiesToV8Object(
 
   Document& document = element->GetDocument();
   StyleResolverState state(document, *element);
-  state.SetStyle(document.GetStyleResolver().CreateComputedStyle());
+  state.SetStyle(document.GetStyleResolver().InitialStyle());
   CSSInterpolationTypesMap map(document.GetPropertyRegistry(), document);
   CSSInterpolationEnvironment environment(map, state);
   value_->GetType().Apply(value_->GetInterpolableValue(),

third_party/blink/renderer/core/css/properties/css_property_test.cc

@@ -40,7 +40,7 @@ class CSSPropertyTest : public PageTestBase {
       const CSSProperty& property,
       const CSSValue& value) {
     StyleResolverState state(GetDocument(), *GetDocument().body());
-    state.SetStyle(GetDocument().GetStyleResolver().CreateComputedStyle());
+    state.SetStyle(GetDocument().GetStyleResolver().InitialStyle());
 
     // The border-style needs to be non-hidden and non-none, otherwise
     // the computed values of border-width properties are always zero.

third_party/blink/renderer/core/css/resolver/matched_properties_cache_test.cc

@@ -70,7 +70,7 @@ class MatchedPropertiesCacheTestCache {
     StyleResolverState state(document_, *document_.body(),
                              nullptr /* StyleRecalcContext */,
                              StyleRequest(&parent_style));
-    state.SetStyle(ComputedStyle::Clone(style));
+    state.SetStyle(style);
     return cache_.Find(key.InnerKey(), state);
   }
 

third_party/blink/renderer/core/css/resolver/style_builder_test.cc

@@ -38,7 +38,7 @@ TEST_F(StyleBuilderTest, WritingModeChangeDirtiesFont) {
       StyleResolverState state(GetDocument(), *GetDocument().body(),
                                nullptr /* StyleRecalcContext */,
                                StyleRequest(parent_style.get()));
-      state.SetStyle(GetDocument().GetStyleResolver().CreateComputedStyle());
+      state.SetStyle(GetDocument().GetStyleResolver().InitialStyle());
 
       // This test assumes that initial 'writing-mode' is not 'vertical-lr'.
       ASSERT_NE(WritingMode::kVerticalLr,
@@ -71,7 +71,7 @@ TEST_F(StyleBuilderTest, TextOrientationChangeDirtiesFont) {
       StyleResolverState state(GetDocument(), *GetDocument().body(),
                                nullptr /* StyleRecalcContext */,
                                StyleRequest(parent_style.get()));
-      state.SetStyle(GetDocument().GetStyleResolver().CreateComputedStyle());
+      state.SetStyle(GetDocument().GetStyleResolver().InitialStyle());
 
       // This test assumes that initial 'text-orientation' is not 'upright'.
       ASSERT_NE(ETextOrientation::kUpright,
@@ -87,22 +87,21 @@ TEST_F(StyleBuilderTest, TextOrientationChangeDirtiesFont) {
 
 TEST_F(StyleBuilderTest, HasExplicitInheritance) {
   auto parent_style = GetDocument().GetStyleResolver().CreateComputedStyle();
-  auto style = GetDocument().GetStyleResolver().CreateComputedStyle();
   StyleResolverState state(GetDocument(), *GetDocument().body(),
                            nullptr /* StyleRecalcContext */,
                            StyleRequest(parent_style.get()));
-  state.SetStyle(style);
-  EXPECT_FALSE(style->HasExplicitInheritance());
+  state.SetStyle(GetDocument().GetStyleResolver().InitialStyle());
+  EXPECT_FALSE(state.StyleBuilder().HasExplicitInheritance());
 
   const CSSValue& inherited = *CSSInheritedValue::Create();
 
   // Flag should not be set for properties which are inherited.
   StyleBuilder::ApplyProperty(GetCSSPropertyColor(), state, inherited);
-  EXPECT_FALSE(style->HasExplicitInheritance());
+  EXPECT_FALSE(state.StyleBuilder().HasExplicitInheritance());
 
   StyleBuilder::ApplyProperty(GetCSSPropertyBackgroundColor(), state,
                               inherited);
-  EXPECT_TRUE(style->HasExplicitInheritance());
+  EXPECT_TRUE(state.StyleBuilder().HasExplicitInheritance());
 }
 
 TEST_F(StyleBuilderTest, GridTemplateAreasApplyOrder) {
@@ -132,7 +131,7 @@ TEST_F(StyleBuilderTest, GridTemplateAreasApplyOrder) {
   scoped_refptr<ComputedStyle> style2;
 
   // grid-template-areas applied first.
-  state.SetStyle(ComputedStyle::Clone(*parent_style));
+  state.SetStyle(*parent_style);
   StyleBuilder::ApplyProperty(grid_template_areas, state,
                               *grid_template_areas_value);
   StyleBuilder::ApplyProperty(grid_template_columns, state,
@@ -142,7 +141,7 @@ TEST_F(StyleBuilderTest, GridTemplateAreasApplyOrder) {
   style1 = state.TakeStyle();
 
   // grid-template-areas applied last.
-  state.SetStyle(ComputedStyle::Clone(*parent_style));
+  state.SetStyle(*parent_style);
   StyleBuilder::ApplyProperty(grid_template_columns, state,
                               *grid_template_columns_value);
   StyleBuilder::ApplyProperty(grid_template_rows, state,

third_party/blink/renderer/core/css/resolver/style_cascade.cc

@@ -175,6 +175,7 @@ void StyleCascade::AddInterpolations(const ActiveInterpolationsMap* map,
 
 void StyleCascade::Apply(CascadeFilter filter) {
   AnalyzeIfNeeded();
+  state_.UpdateLengthConversionData();
 
   CascadeResolver resolver(filter, ++generation_);
 

third_party/blink/renderer/core/css/resolver/style_cascade_test.cc

@@ -226,7 +226,7 @@ class TestCascade {
   Element* Body() const { return GetDocument().body(); }
 
   static StyleResolverState& InitState(StyleResolverState& state) {
-    state.SetStyle(InitialStyle(state.GetDocument()));
+    state.SetStyle(*InitialStyle(state.GetDocument()));
     state.SetParentStyle(InitialStyle(state.GetDocument()));
     return state;
   }

third_party/blink/renderer/core/css/resolver/style_resolver.cc

@@ -1093,23 +1093,20 @@ void StyleResolver::ApplyInheritance(Element& element,
     // entirely (leaving the scoped_refptr untouched). The bad news is that if
     // the element has rules but no matched properties, we currently clone.
 
-    state.SetStyle(ComputedStyle::Clone(*state.ParentStyle()));
+    state.SetStyle(*state.ParentStyle());
   } else {
     // We use a different initial_style for img elements to match the overrides
     // in html.css. This avoids allocation overhead from copy-on-write when
     // these properties are set only via UA styles. The overhead shows up on
     // motionmark which stress tests this code. See crbub.com/1369454 for
     // details.
-    ComputedStyleBuilder builder(IsA<HTMLImageElement>(element)
-                                     ? *initial_style_for_img_
-                                     : *initial_style_);
-
-    builder.InheritFrom(
+    state.SetStyle(IsA<HTMLImageElement>(element) ? *initial_style_for_img_
+                                                  : *initial_style_);
+    state.StyleBuilder().InheritFrom(
         *state.ParentStyle(),
         (!style_request.IsPseudoStyleRequest() && IsAtShadowBoundary(&element))
             ? ComputedStyleBuilder::kAtShadowBoundary
             : ComputedStyleBuilder::kNotAtShadowBoundary);
-    state.SetStyle(builder.TakeStyle());
 
     // contenteditable attribute (implemented by -webkit-user-modify) should
     // be propagated from shadow host to distributed node.
@@ -1131,9 +1128,9 @@ void StyleResolver::InitStyleAndApplyInheritance(
   if (AllowsInheritance(style_request, state.ParentStyle())) {
     ApplyInheritance(element, style_request, state);
   } else {
-    state.SetStyle(InitialStyleForElement());
-    state.SetParentStyle(
-        ComputedStyle::Clone(*state.StyleBuilder().InternalStyle()));
+    scoped_refptr<const ComputedStyle> initial_style = InitialStyleForElement();
+    state.SetStyle(*initial_style);
+    state.SetParentStyle(initial_style);
     state.SetLayoutParentStyle(state.ParentStyle());
     if (!style_request.IsPseudoStyleRequest() &&
         element != GetDocument().documentElement()) {
@@ -1513,7 +1510,7 @@ void StyleResolver::ApplyBaseStyle(
                                animation_base_computed_style, *style_snapshot));
 #endif
 
-    state.SetStyle(ComputedStyle::Clone(*animation_base_computed_style));
+    state.SetStyle(*animation_base_computed_style);
     state.StyleBuilder().SetBaseData(
         scoped_refptr<StyleBaseData>(GetBaseData(state)));
     state.StyleBuilder().SetStyleType(style_request.pseudo_id);
@@ -1532,7 +1529,7 @@ void StyleResolver::ApplyBaseStyle(
     // We are in a situation where we can reuse the old style
     // and just apply the element's inline style on top of it
     // (see the function comment).
-    state.SetStyle(ComputedStyle::Clone(*element->GetComputedStyle()));
+    state.SetStyle(*element->GetComputedStyle());
 
     const CSSPropertyValueSet* inline_style = element->InlineStyle();
     if (inline_style) {
@@ -1619,7 +1616,7 @@ CompositorKeyframeValue* StyleResolver::CreateCompositorKeyframeValueSnapshot(
   StyleResolverState state(element.GetDocument(), element,
                            nullptr /* StyleRecalcContext */,
                            StyleRequest(parent_style));
-  state.SetStyle(ComputedStyle::Clone(base_style));
+  state.SetStyle(base_style);
   if (value) {
     STACK_UNINITIALIZED StyleCascade cascade(state);
     auto* set =
@@ -1645,14 +1642,12 @@ scoped_refptr<const ComputedStyle> StyleResolver::StyleForPage(
     return initial_style;
   }
 
+  const ComputedStyle* document_style = GetDocument().GetComputedStyle();
   StyleResolverState state(GetDocument(), *GetDocument().documentElement(),
                            nullptr /* StyleRecalcContext */,
                            StyleRequest(initial_style.get()));
-
-  const ComputedStyle* document_style = GetDocument().GetComputedStyle();
-  ComputedStyleBuilder builder = CreateComputedStyleBuilder();
-  builder.InheritFrom(*document_style);
-  state.SetStyle(builder.TakeStyle());
+  state.SetStyle(*initial_style);
+  state.StyleBuilder().InheritFrom(*document_style);
 
   STACK_UNINITIALIZED StyleCascade cascade(state);
 
@@ -1765,7 +1760,7 @@ StyleRuleList* StyleResolver::StyleRulesForElement(Element* element,
 HeapHashMap<CSSPropertyName, Member<const CSSValue>>
 StyleResolver::CascadedValuesForElement(Element* element, PseudoId pseudo_id) {
   StyleResolverState state(GetDocument(), *element);
-  state.SetStyle(CreateComputedStyle());
+  state.SetStyle(InitialStyle());
 
   STACK_UNINITIALIZED StyleCascade cascade(state);
   ElementRuleCollector collector(state.ElementContext(),
@@ -2193,7 +2188,7 @@ const CSSValue* StyleResolver::ComputeValue(
   const ComputedStyle* base_style = element->GetComputedStyle();
   StyleResolverState state(element->GetDocument(), *element);
   STACK_UNINITIALIZED StyleCascade cascade(state);
-  state.SetStyle(ComputedStyle::Clone(*base_style));
+  state.SetStyle(*base_style);
   auto* set =
       MakeGarbageCollected<MutableCSSPropertyValueSet>(state.GetParserMode());
   set->SetProperty(property_name, value);
@@ -2223,7 +2218,7 @@ FilterOperations StyleResolver::ComputeFilterOperations(
                            nullptr /* StyleRecalcContext */,
                            StyleRequest(parent.get()));
 
-  state.SetStyle(ComputedStyle::Clone(*parent));
+  state.SetStyle(*parent);
 
   StyleBuilder::ApplyProperty(GetCSSPropertyFilter(), state,
                               filter_value.EnsureScopedValue(&GetDocument()));
@@ -2267,7 +2262,7 @@ StyleResolver::BeforeChangeStyleForTransitionUpdate(
     ActiveInterpolationsMap& transition_interpolations) {
   StyleResolverState state(GetDocument(), element);
   STACK_UNINITIALIZED StyleCascade cascade(state);
-  state.SetStyle(ComputedStyle::Clone(base_style));
+  state.SetStyle(base_style);
 
   // Various property values may depend on the parent style. A valid parent
   // style is required, even if animating the root element, in order to
@@ -2391,7 +2386,7 @@ StyleRuleList* StyleResolver::CollectMatchingRulesFromRuleSet(
 // thread. If you add/remove properties here, make sure they are also properly
 // handled by FontStyleResolver.
 Font StyleResolver::ComputeFont(Element& element,
-                                ComputedStyle& style,
+                                const ComputedStyle& style,
                                 const CSSPropertyValueSet& property_set) {
   static const CSSProperty* properties[6] = {
       &GetCSSPropertyFontSize(),        &GetCSSPropertyFontFamily(),
@@ -2403,7 +2398,7 @@ Font StyleResolver::ComputeFont(Element& element,
   StyleResolverState state(GetDocument(), element,
                            nullptr /* StyleRecalcContext */,
                            StyleRequest(&style));
-  state.SetStyle(&style);
+  state.SetStyle(style);
   if (const ComputedStyle* parent_style = element.GetComputedStyle()) {
     state.SetParentStyle(parent_style);
   }
@@ -2781,7 +2776,7 @@ scoped_refptr<const ComputedStyle> StyleResolver::StyleForFormattedText(
       GetDocument(), EnsureElementForFormattedText(),
       nullptr /* StyleRecalcContext */,
       StyleRequest{parent_style ? parent_style : &InitialStyle()});
-  state.SetStyle(builder.TakeStyle());
+  state.SetStyle(*builder.TakeStyle());
 
   // Use StyleCascade to apply inheritance in the correct order.
   STACK_UNINITIALIZED StyleCascade cascade(state);
@@ -2905,7 +2900,7 @@ scoped_refptr<const ComputedStyle> StyleResolver::ResolvePositionFallbackStyle(
 
   StyleRuleTry* try_rule = position_fallback_rule->TryRules()[index];
   StyleResolverState state(GetDocument(), element);
-  state.SetStyle(ComputedStyle::Clone(base_style));
+  state.SetStyle(base_style);
   const CSSPropertyValueSet& properties = try_rule->Properties();
 
   STACK_UNINITIALIZED StyleCascade cascade(state);

third_party/blink/renderer/core/css/resolver/style_resolver.h

@@ -174,7 +174,7 @@ class CORE_EXPORT StyleResolver final : public GarbageCollected<StyleResolver> {
 
   Element* FindContainerForElement(Element*, const ContainerSelector&);
 
-  Font ComputeFont(Element&, ComputedStyle&, const CSSPropertyValueSet&);
+  Font ComputeFont(Element&, const ComputedStyle&, const CSSPropertyValueSet&);
 
   // FIXME: Rename to reflect the purpose, like didChangeFontSize or something.
   void InvalidateMatchedPropertiesCache();

third_party/blink/renderer/core/css/resolver/style_resolver_state.cc

@@ -103,23 +103,17 @@ bool StyleResolverState::IsInheritedForUnset(
   return property.IsInherited() || UsesHighlightPseudoInheritance();
 }
 
-void StyleResolverState::SetStyle(scoped_refptr<ComputedStyle> style) {
-  // FIXME: Improve RAII of StyleResolverState to remove this function.
-  style_builder_.SetStyle(std::move(style));
-  UpdateLengthConversionData();
-}
-
 scoped_refptr<ComputedStyle> StyleResolverState::TakeStyle() {
   if (had_no_matched_properties_ &&
       pseudo_request_type_ == StyleRequest::kForRenderer) {
     return nullptr;
   }
-  return style_builder_.TakeStyle();
+  return style_builder_->TakeStyle();
 }
 
 void StyleResolverState::UpdateLengthConversionData() {
   css_to_length_conversion_data_ = CSSToLengthConversionData(
-      style_builder_, ParentStyle(), RootElementStyle(),
+      *style_builder_, ParentStyle(), RootElementStyle(),
       GetDocument().GetLayoutView(),
       CSSToLengthConversionData::ContainerSizes(container_unit_context_),
       StyleBuilder().EffectiveZoom(), length_conversion_flags_);
@@ -134,7 +128,7 @@ CSSToLengthConversionData StyleResolverState::UnzoomedLengthConversionData(
                                                   root_font_style);
   CSSToLengthConversionData::LineHeightSize line_height_size(
       ParentStyle() ? ParentStyle()->GetFontSizeStyle()
-                    : style_builder_.GetFontSizeStyle(),
+                    : style_builder_->GetFontSizeStyle(),
       root_font_style);
   CSSToLengthConversionData::ViewportSize viewport_size(
       GetDocument().GetLayoutView());
@@ -151,7 +145,7 @@ CSSToLengthConversionData StyleResolverState::FontSizeConversionData() {
 }
 
 CSSToLengthConversionData StyleResolverState::UnzoomedLengthConversionData() {
-  return UnzoomedLengthConversionData(style_builder_.GetFontSizeStyle());
+  return UnzoomedLengthConversionData(style_builder_->GetFontSizeStyle());
 }
 
 void StyleResolverState::SetParentStyle(
@@ -170,7 +164,7 @@ void StyleResolverState::LoadPendingResources() {
   if (pseudo_request_type_ == StyleRequest::kForComputedStyle ||
       (ParentStyle() && ParentStyle()->IsEnsuredInDisplayNone()) ||
       (StyleBuilder().Display() == EDisplay::kNone &&
-       !GetElement().LayoutObjectIsNeeded(style_builder_.GetDisplayStyle())) ||
+       !GetElement().LayoutObjectIsNeeded(style_builder_->GetDisplayStyle())) ||
       StyleBuilder().IsEnsuredOutsideFlatTree()) {
     return;
   }
@@ -257,14 +251,14 @@ const CSSValue& StyleResolverState::ResolveLightDarkPair(
 void StyleResolverState::UpdateFont() {
   GetFontBuilder().CreateFont(StyleBuilder(), ParentStyle());
   SetConversionFontSizes(CSSToLengthConversionData::FontSizes(
-      style_builder_.GetFontSizeStyle(), RootElementStyle()));
+      style_builder_->GetFontSizeStyle(), RootElementStyle()));
   SetConversionZoom(StyleBuilder().EffectiveZoom());
 }
 
 void StyleResolverState::UpdateLineHeight() {
   css_to_length_conversion_data_.SetLineHeightSize(
       CSSToLengthConversionData::LineHeightSize(
-          style_builder_.GetFontSizeStyle(),
+          style_builder_->GetFontSizeStyle(),
           GetDocument().documentElement()->GetComputedStyle()));
 }
 

third_party/blink/renderer/core/css/resolver/style_resolver_state.h

@@ -86,9 +86,13 @@ class CORE_EXPORT StyleResolverState {
     return element_context_;
   }
 
-  void SetStyle(scoped_refptr<ComputedStyle>);
-  ComputedStyleBuilder& StyleBuilder() { return style_builder_; }
-  const ComputedStyleBuilder& StyleBuilder() const { return style_builder_; }
+  void SetStyle(const ComputedStyle& style) {
+    // FIXME: Improve RAII of StyleResolverState to remove this function.
+    style_builder_ = ComputedStyleBuilder(style);
+    UpdateLengthConversionData();
+  }
+  ComputedStyleBuilder& StyleBuilder() { return *style_builder_; }
+  const ComputedStyleBuilder& StyleBuilder() const { return *style_builder_; }
   scoped_refptr<ComputedStyle> TakeStyle();
 
   const CSSToLengthConversionData& CssToLengthConversionData() const {
@@ -203,15 +207,16 @@ class CORE_EXPORT StyleResolverState {
   // Update computed line-height and font used for 'lh' unit resolution.
   void UpdateLineHeight();
 
- private:
   void UpdateLengthConversionData();
+
+ private:
   CSSToLengthConversionData UnzoomedLengthConversionData(const FontSizeStyle&);
 
   ElementResolveContext element_context_;
   Document* document_;
 
   // The primary output for each element's style resolve.
-  ComputedStyleBuilder style_builder_;
+  absl::optional<ComputedStyleBuilder> style_builder_;
 
   CSSToLengthConversionData::Flags length_conversion_flags_ = 0;
   CSSToLengthConversionData css_to_length_conversion_data_;