Skip to content

Commit

Permalink
[StyleBuilder] Remove ComputedStyle from CSSToLengthConversionData
Browse files Browse the repository at this point in the history
CSSToLengthConversionData currently does a const_cast of an incoming
ComputedStyle in order to set certain flags on that ComputedStyle.
This is bad, because CSSToLengthConversionData is used in several
situations outside of style recalc, i.e. in situations where it's too
late to modify the ComputedStyle.

It's also a major hurdle for the StyleBuilder project, because just
replacing the ComputedStyle pointer with a ComputedStyleBuilder pointer
isn't possible: we don't have a ComputedStyleBuilder at all outside
of style resolution.

This CL addresses this by propagating the unit flags to
CSSToLengthConversionData's call site.

Bug: 1377295
Change-Id: I9d162aebf813418c15a494a1d18581b75272aa10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4107634
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084547}
  • Loading branch information
andruud authored and Chromium LUCI CQ committed Dec 16, 2022
1 parent ce24f19 commit ea1ff91
Show file tree
Hide file tree
Showing 16 changed files with 257 additions and 101 deletions.
5 changes: 4 additions & 1 deletion third_party/blink/renderer/core/animation/view_timeline.cc
Expand Up @@ -159,11 +159,14 @@ Length InsetValueToLength(const CSSValue* inset_value,
if (inset_value->IsPrimitiveValue()) {
ElementResolveContext element_resolve_context(*subject);
Document& document = subject->GetDocument();
// Flags can be ignored, because we re-resolve any value that's not px or
// percentage, see IsStyleDependent.
CSSToLengthConversionData::Flags ignored_flags = 0;
CSSToLengthConversionData length_conversion_data(
subject->GetComputedStyle(), element_resolve_context.ParentStyle(),
element_resolve_context.RootElementStyle(), document.GetLayoutView(),
CSSToLengthConversionData::ContainerSizes(subject),
subject->GetComputedStyle()->EffectiveZoom());
subject->GetComputedStyle()->EffectiveZoom(), ignored_flags);

return DynamicTo<CSSPrimitiveValue>(inset_value)
->ConvertToLength(length_conversion_data);
Expand Down
5 changes: 4 additions & 1 deletion third_party/blink/renderer/core/css/css_gradient_value.cc
Expand Up @@ -135,9 +135,12 @@ scoped_refptr<Image> CSSGradientValue::GetImage(
// We need to create an image.
const ComputedStyle* root_style =
document.documentElement()->GetComputedStyle();

// TODO(crbug.com/947377): Conversion is not supposed to happen here.
CSSToLengthConversionData::Flags ignored_flags = 0;
CSSToLengthConversionData conversion_data(
&style, &style, root_style, document.GetLayoutView(), container_sizes,
style.EffectiveZoom());
style.EffectiveZoom(), ignored_flags);

scoped_refptr<Gradient> gradient;
switch (GetClassType()) {
Expand Down
Expand Up @@ -94,9 +94,11 @@ TEST(CSSCalculationValue, AccumulatePixelsAndPercent) {
ComputedStyleBuilder builder(*ComputedStyle::CreateInitialStyleSingleton());
builder.SetEffectiveZoom(5);
scoped_refptr<const ComputedStyle> style = builder.TakeStyle();
CSSToLengthConversionData::Flags ignored_flags = 0;
CSSToLengthConversionData conversion_data(
style.get(), style.get(), style.get(), nullptr,
CSSToLengthConversionData::ContainerSizes(), style->EffectiveZoom());
CSSToLengthConversionData::ContainerSizes(), style->EffectiveZoom(),
ignored_flags);

TestAccumulatePixelsAndPercent(
conversion_data,
Expand Down
Expand Up @@ -60,11 +60,6 @@ absl::optional<double> FindSizeForContainerAxis(PhysicalAxes requested_axis,
return evaluator->Height();
}

void SetHasContainerRelativeUnits(const ComputedStyle* style) {
const_cast<ComputedStyle*>(style)->SetHasContainerRelativeUnits();
const_cast<ComputedStyle*>(style)->SetDependsOnSizeContainerQueries(true);
}

} // namespace

CSSToLengthConversionData::FontSizes::FontSizes(float em,
Expand Down Expand Up @@ -193,136 +188,117 @@ void CSSToLengthConversionData::ContainerSizes::CacheSizeIfNeeded(
}

CSSToLengthConversionData::CSSToLengthConversionData(
const ComputedStyle* element_style,
WritingMode writing_mode,
const FontSizes& font_sizes,
const LineHeightSize& line_height_size,
const ViewportSize& viewport_size,
const ContainerSizes& container_sizes,
float zoom)
float zoom,
Flags& flags)
: CSSLengthResolver(
ClampTo<float>(zoom, std::numeric_limits<float>::denorm_min())),
style_(element_style),
writing_mode_(writing_mode),
font_sizes_(font_sizes),
line_height_size_(line_height_size),
viewport_size_(viewport_size),
container_sizes_(container_sizes) {}
container_sizes_(container_sizes),
flags_(&flags) {}

CSSToLengthConversionData::CSSToLengthConversionData(
const ComputedStyle* element_style,
const ComputedStyle* parent_style,
const ComputedStyle* root_style,
const LayoutView* layout_view,
const ContainerSizes& container_sizes,
float zoom)
float zoom,
Flags& flags)
: CSSToLengthConversionData(
element_style,
element_style->GetWritingMode(),
FontSizes(element_style, root_style),
LineHeightSize(parent_style ? *parent_style : *element_style),
ViewportSize(layout_view),
container_sizes,
zoom) {}
zoom,
flags) {}

float CSSToLengthConversionData::EmFontSize(float zoom) const {
// FIXME: Remove style_ from this class. Plumb viewport and font unit
// information through as output parameters on functions involved in length
// resolution.
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasEmUnits();
SetFlag(Flag::kEm);
return font_sizes_.Em(zoom);
}

float CSSToLengthConversionData::RemFontSize(float zoom) const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasRemUnits();
SetFlag(Flag::kRem);
return font_sizes_.Rem(zoom);
}

float CSSToLengthConversionData::ExFontSize(float zoom) const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasGlyphRelativeUnits();
SetFlag(Flag::kGlyphRelative);
return font_sizes_.Ex(zoom);
}

float CSSToLengthConversionData::ChFontSize(float zoom) const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasGlyphRelativeUnits();
SetFlag(Flag::kGlyphRelative);
return font_sizes_.Ch(zoom);
}

float CSSToLengthConversionData::IcFontSize(float zoom) const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasGlyphRelativeUnits();
SetFlag(Flag::kGlyphRelative);
return font_sizes_.Ic(zoom);
}

float CSSToLengthConversionData::LineHeight(float zoom) const {
if (style_) {
const_cast<ComputedStyle*>(style_)->SetHasGlyphRelativeUnits();
const_cast<ComputedStyle*>(style_)->SetHasLineHeightRelativeUnits();
}
SetFlag(Flag::kGlyphRelative);
SetFlag(Flag::kLineHeightRelative);
return line_height_size_.Lh(zoom);
}

double CSSToLengthConversionData::ViewportWidth() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasStaticViewportUnits();
SetFlag(Flag::kStaticViewport);
return viewport_size_.LargeWidth();
}

double CSSToLengthConversionData::ViewportHeight() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasStaticViewportUnits();
SetFlag(Flag::kStaticViewport);
return viewport_size_.LargeHeight();
}

double CSSToLengthConversionData::SmallViewportWidth() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasStaticViewportUnits();
SetFlag(Flag::kStaticViewport);
return viewport_size_.SmallWidth();
}

double CSSToLengthConversionData::SmallViewportHeight() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasStaticViewportUnits();
SetFlag(Flag::kStaticViewport);
return viewport_size_.SmallHeight();
}

double CSSToLengthConversionData::LargeViewportWidth() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasStaticViewportUnits();
SetFlag(Flag::kStaticViewport);
return viewport_size_.LargeWidth();
}

double CSSToLengthConversionData::LargeViewportHeight() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasStaticViewportUnits();
SetFlag(Flag::kStaticViewport);
return viewport_size_.LargeHeight();
}

double CSSToLengthConversionData::DynamicViewportWidth() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasDynamicViewportUnits();
SetFlag(Flag::kDynamicViewport);
return viewport_size_.DynamicWidth();
}

double CSSToLengthConversionData::DynamicViewportHeight() const {
if (style_)
const_cast<ComputedStyle*>(style_)->SetHasDynamicViewportUnits();
SetFlag(Flag::kDynamicViewport);
return viewport_size_.DynamicHeight();
}

double CSSToLengthConversionData::ContainerWidth() const {
if (style_)
SetHasContainerRelativeUnits(style_);
SetFlag(Flag::kContainerRelative);
return container_sizes_.Width().value_or(SmallViewportWidth());
}

double CSSToLengthConversionData::ContainerHeight() const {
if (style_)
SetHasContainerRelativeUnits(style_);
SetFlag(Flag::kContainerRelative);
return container_sizes_.Height().value_or(SmallViewportHeight());
}

Expand All @@ -332,7 +308,7 @@ WritingMode CSSToLengthConversionData::GetWritingMode() const {

CSSToLengthConversionData::ContainerSizes
CSSToLengthConversionData::PreCachedContainerSizesCopy() const {
SetHasContainerRelativeUnits(style_);
SetFlag(Flag::kContainerRelative);
return container_sizes_.PreCachedCopy();
}

Expand Down
Expand Up @@ -176,20 +176,43 @@ class CORE_EXPORT CSSToLengthConversionData : public CSSLengthResolver {
mutable absl::optional<double> cached_height_;
};

using Flags = uint8_t;

// Flags represent the units seen in a conversion. They are used for targeted
// invalidation, e.g. when root font-size changes, only elements dependent on
// rem units are recalculated.
enum class Flag : Flags {
// em
kEm = 1u << 0,
// rem
kRem = 1u << 1,
// ex, ch, ic, lh
kGlyphRelative = 1u << 2,
// lh
kLineHeightRelative = 1u << 3,
// sv*, lv*, v*
kStaticViewport = 1u << 4,
// dv*
kDynamicViewport = 1u << 5,
// cq*
kContainerRelative = 1u << 6,
};

CSSToLengthConversionData() : CSSLengthResolver(1 /* zoom */) {}
CSSToLengthConversionData(const ComputedStyle* element_style,
WritingMode,
CSSToLengthConversionData(WritingMode,
const FontSizes&,
const LineHeightSize&,
const ViewportSize&,
const ContainerSizes&,
float zoom);
float zoom,
Flags&);
CSSToLengthConversionData(const ComputedStyle* element_style,
const ComputedStyle* parent_style,
const ComputedStyle* root_style,
const LayoutView*,
const ContainerSizes&,
float zoom);
float zoom,
Flags&);

float EmFontSize(float zoom) const override;
float RemFontSize(float zoom) const override;
Expand Down Expand Up @@ -221,21 +244,28 @@ class CORE_EXPORT CSSToLengthConversionData : public CSSLengthResolver {
ContainerSizes PreCachedContainerSizesCopy() const;

CSSToLengthConversionData CopyWithAdjustedZoom(float new_zoom) const {
return CSSToLengthConversionData(style_, writing_mode_, font_sizes_,
DCHECK(flags_);
return CSSToLengthConversionData(writing_mode_, font_sizes_,
line_height_size_, viewport_size_,
container_sizes_, new_zoom);
container_sizes_, new_zoom, *flags_);
}
CSSToLengthConversionData Unzoomed() const {
return CopyWithAdjustedZoom(1.0f);
}

private:
const ComputedStyle* style_ = nullptr;
void SetFlag(Flag flag) const {
if (flags_) {
*flags_ |= static_cast<Flags>(flag);
}
}

WritingMode writing_mode_ = WritingMode::kHorizontalTb;
FontSizes font_sizes_;
LineHeightSize line_height_size_;
ViewportSize viewport_size_;
ContainerSizes container_sizes_;
mutable Flags* flags_ = nullptr;
};

class ScopedCSSToLengthConversionData : public CSSToLengthConversionData {
Expand Down

0 comments on commit ea1ff91

Please sign in to comment.