Commit fbe0aaf8 authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot

Revert "Pass CSSValue TreeScope to StyleBuilder::ApplyProperty"

This reverts commit 4d32f3cb.

Reason for revert: crbug.com/1139337

Original change's description:
> Pass CSSValue TreeScope to StyleBuilder::ApplyProperty
>
> Wrap CSSValue and the corresponding TreeScope from MatchResult in a
> ScopedCSSValue passing it to StyleBuilder.
>
> Currently use a nullptr TreeScope in the cases where we don't have one
> to look up tree-scoped references. We do not support such names/
> references in UA stylesheets. We currently support @keyframes in user
> stylesheets, but collect them from the StyleEngine regardless of which
> scope the animation-name came from. It would be possible to have a
> TreeScope for user sheets (and even UA sheets) and use
> ScopedStyleResolvers instead of extra structures in StyleEngine. For now
> pass null TreeScopes for UA and user style values.
>
> Bug: 336876
>
> Change-Id: I4c5af46bde94b9fda41d676982d9e9386b652538
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2455609
> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
> Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> Commit-Queue: Rune Lillesveen <futhark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#816999}

TBR=fserb@chromium.org,futhark@chromium.org,andruud@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 336876
Change-Id: I223a24e0e93ed80dbfd9a7f9ba9b2ced10acb93c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480644Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818040}
parent adf80f27
......@@ -7,7 +7,6 @@
#include "third_party/blink/renderer/core/animation/css_interpolation_environment.h"
#include "third_party/blink/renderer/core/animation/string_keyframe.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
namespace blink {
......@@ -49,9 +48,7 @@ void CSSDefaultInterpolationType::Apply(
StyleBuilder::ApplyProperty(
GetProperty().GetCSSPropertyName(),
To<CSSInterpolationEnvironment>(environment).GetState(),
ScopedCSSValue(*To<CSSDefaultNonInterpolableValue>(non_interpolable_value)
->CssValue(),
nullptr));
*To<CSSDefaultNonInterpolableValue>(non_interpolable_value)->CssValue());
}
} // namespace blink
......@@ -24,7 +24,6 @@
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_cascade.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/style/data_equivalency.h"
#include "third_party/blink/renderer/core/style_property_shorthand.h"
......@@ -337,7 +336,7 @@ void CSSInterpolationType::ApplyCustomPropertyValue(
const CSSValue* value = MakeGarbageCollected<CSSCustomPropertyDeclaration>(
property.CustomPropertyName(), std::move(variable_data));
StyleBuilder::ApplyProperty(GetProperty().GetCSSPropertyName(), state,
ScopedCSSValue(*value, nullptr));
*value);
}
} // namespace blink
......@@ -13,7 +13,6 @@
#include "third_party/blink/renderer/core/css/css_identifier_value.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/platform/geometry/length_functions.h"
......@@ -146,9 +145,8 @@ void CSSLengthInterpolationType::ApplyStandardPropertyValue(
Length before;
Length after;
DCHECK(LengthPropertyFunctions::GetLength(CssProperty(), style, before));
StyleBuilder::ApplyProperty(
GetProperty().GetCSSProperty(), state,
ScopedCSSValue(*CSSValue::Create(length, zoom), nullptr));
StyleBuilder::ApplyProperty(GetProperty().GetCSSProperty(), state,
*CSSValue::Create(length, zoom));
DCHECK(LengthPropertyFunctions::GetLength(CssProperty(), style, after));
DCHECK(before.IsSpecified());
DCHECK(after.IsSpecified());
......@@ -164,9 +162,8 @@ void CSSLengthInterpolationType::ApplyStandardPropertyValue(
#endif
return;
}
StyleBuilder::ApplyProperty(
GetProperty().GetCSSProperty(), state,
ScopedCSSValue(*CSSValue::Create(length, zoom), nullptr));
StyleBuilder::ApplyProperty(GetProperty().GetCSSProperty(), state,
*CSSValue::Create(length, zoom));
}
} // namespace blink
......@@ -12,7 +12,6 @@
#include "third_party/blink/renderer/core/css/css_numeric_literal_value.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
namespace blink {
......@@ -111,10 +110,8 @@ void CSSNumberInterpolationType::ApplyStandardPropertyValue(
clamped_number)) {
StyleBuilder::ApplyProperty(
GetProperty().GetCSSProperty(), state,
ScopedCSSValue(
*CSSNumericLiteralValue::Create(
clamped_number, CSSPrimitiveValue::UnitType::kNumber),
nullptr));
*CSSNumericLiteralValue::Create(clamped_number,
CSSPrimitiveValue::UnitType::kNumber));
}
}
......
......@@ -14,7 +14,6 @@
#include "third_party/blink/renderer/core/css/property_registration.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_cascade.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
namespace blink {
......@@ -117,10 +116,8 @@ void CSSVarCycleInterpolationType::Apply(
StyleBuilder::ApplyProperty(
GetProperty().GetCSSPropertyName(),
To<CSSInterpolationEnvironment>(environment).GetState(),
ScopedCSSValue(
*MakeGarbageCollected<CSSCustomPropertyDeclaration>(
GetProperty().CustomPropertyName(), CSSValueID::kUnset),
nullptr));
*MakeGarbageCollected<CSSCustomPropertyDeclaration>(
GetProperty().CustomPropertyName(), CSSValueID::kUnset));
}
} // namespace blink
......@@ -533,7 +533,6 @@ blink_core_sources("css") {
"rule_feature_set.h",
"rule_set.cc",
"rule_set.h",
"scoped_css_value.h",
"select_rule_feature_set.cc",
"select_rule_feature_set.h",
"selector_checker.cc",
......
......@@ -10,7 +10,6 @@
#include "third_party/blink/renderer/core/css/properties/css_property_ref.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/html/html_element.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/style/data_equivalency.h"
......@@ -43,8 +42,7 @@ class CSSPropertyTest : public PageTestBase {
state.Style()->SetBorderRightStyle(EBorderStyle::kSolid);
state.Style()->SetBorderTopStyle(EBorderStyle::kSolid);
StyleBuilder::ApplyProperty(property, state,
ScopedCSSValue(value, &GetDocument()));
StyleBuilder::ApplyProperty(property, state, value);
return state.TakeStyle();
}
};
......
......@@ -186,8 +186,4 @@ CSSPropertyValueSet::PropertyReference CascadeExpansion::PropertyAt(
return matched_properties_.properties->PropertyAt(index_);
}
uint16_t CascadeExpansion::TreeOrder() const {
return matched_properties_.types_.tree_order;
}
} // namespace blink
......@@ -98,7 +98,6 @@ class CORE_EXPORT CascadeExpansion {
return PropertyAt(index_).Value();
}
inline CascadePriority Priority() const { return priority_; }
uint16_t TreeOrder() const;
private:
static bool IsAffectedByAll(CSSPropertyID);
......
......@@ -48,29 +48,27 @@
#include "third_party/blink/renderer/core/css/properties/longhands/variable.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
namespace blink {
void StyleBuilder::ApplyProperty(const CSSPropertyName& name,
StyleResolverState& state,
const ScopedCSSValue& scoped_value) {
const CSSValue& value) {
CSSPropertyRef ref(name, state.GetDocument());
DCHECK(ref.IsValid());
ApplyProperty(ref.GetProperty(), state, scoped_value);
ApplyProperty(ref.GetProperty(), state, value);
}
void StyleBuilder::ApplyProperty(const CSSProperty& property,
StyleResolverState& state,
const ScopedCSSValue& scoped_value) {
const CSSValue& value) {
DCHECK(!Variable::IsStaticInstance(property))
<< "Please use a CustomProperty instance to apply custom properties";
CSSPropertyID id = property.PropertyID();
bool is_inherited = property.IsInherited();
const CSSValue& value = scoped_value.GetCSSValue();
// These values must be resolved by StyleCascade before application:
DCHECK(!value.IsVariableReferenceValue());
......
......@@ -39,7 +39,7 @@
namespace blink {
class CSSPropertyName;
class ScopedCSSValue;
class CSSValue;
class StyleResolverState;
class CORE_EXPORT StyleBuilder {
......@@ -52,7 +52,7 @@ class CORE_EXPORT StyleBuilder {
// CustomProperty instance is created to carry out the application.
static void ApplyProperty(const CSSPropertyName&,
StyleResolverState&,
const ScopedCSSValue&);
const CSSValue&);
// Apply a property/value pair to the ComputedStyle.
//
......@@ -61,7 +61,7 @@ class CORE_EXPORT StyleBuilder {
// instance. See Variable::IsStaticInstance.
static void ApplyProperty(const CSSProperty&,
StyleResolverState&,
const ScopedCSSValue&);
const CSSValue&);
};
} // namespace blink
......
......@@ -7,7 +7,6 @@
#include "third_party/blink/renderer/core/css/css_inherited_value.h"
#include "third_party/blink/renderer/core/css/css_initial_value.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/html/html_element.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"
......@@ -42,8 +41,7 @@ TEST_F(StyleBuilderTest, WritingModeChangeDirtiesFont) {
state.SetStyle(style);
ASSERT_FALSE(state.GetFontBuilder().FontDirty());
StyleBuilder::ApplyProperty(*property, state,
ScopedCSSValue(*value, &GetDocument()));
StyleBuilder::ApplyProperty(*property, state, *value);
EXPECT_TRUE(state.GetFontBuilder().FontDirty());
}
}
......@@ -74,8 +72,7 @@ TEST_F(StyleBuilderTest, TextOrientationChangeDirtiesFont) {
state.SetStyle(style);
ASSERT_FALSE(state.GetFontBuilder().FontDirty());
StyleBuilder::ApplyProperty(*property, state,
ScopedCSSValue(*value, &GetDocument()));
StyleBuilder::ApplyProperty(*property, state, *value);
EXPECT_TRUE(state.GetFontBuilder().FontDirty());
}
}
......@@ -89,14 +86,13 @@ TEST_F(StyleBuilderTest, HasExplicitInheritance) {
state.SetStyle(style);
EXPECT_FALSE(style->HasExplicitInheritance());
ScopedCSSValue inherited(*CSSInheritedValue::Create(), &GetDocument());
// Flag should not be set for properties which are inherited.
StyleBuilder::ApplyProperty(GetCSSPropertyColor(), state, inherited);
StyleBuilder::ApplyProperty(GetCSSPropertyColor(), state,
*CSSInheritedValue::Create());
EXPECT_FALSE(style->HasExplicitInheritance());
StyleBuilder::ApplyProperty(GetCSSPropertyBackgroundColor(), state,
inherited);
*CSSInheritedValue::Create());
EXPECT_TRUE(style->HasExplicitInheritance());
}
......
......@@ -28,7 +28,6 @@
#include "third_party/blink/renderer/core/css/resolver/cascade_resolver.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/css/style_engine.h"
#include "third_party/blink/renderer/core/dom/shadow_root.h"
#include "third_party/blink/renderer/core/frame/web_feature.h"
......@@ -81,14 +80,6 @@ const CSSValue* ValueAt(const MatchResult& result, uint32_t position) {
return &set->PropertyAt(declaration_index).Value();
}
const TreeScope& TreeScopeAt(const MatchResult& result, uint32_t position) {
size_t matched_properties_index = DecodeMatchedPropertiesIndex(position);
const MatchedProperties& properties =
result.GetMatchedProperties()[matched_properties_index];
DCHECK_EQ(properties.types_.origin, CascadeOrigin::kAuthor);
return result.ScopeFromTreeOrder(properties.types_.tree_order);
}
PropertyHandle ToPropertyHandle(const CSSProperty& property,
CascadePriority priority) {
uint32_t position = priority.GetPosition();
......@@ -406,14 +397,7 @@ void StyleCascade::ApplyMatchResult(CascadeResolver& resolver) {
*p = priority;
CascadeOrigin origin = priority.GetOrigin();
const CSSValue* value = Resolve(property, e.Value(), origin, resolver);
// TODO(futhark): Use a user scope TreeScope to support tree-scoped names
// for animations in user stylesheets.
const TreeScope* tree_scope =
origin == CascadeOrigin::kAuthor
? &match_result_.ScopeFromTreeOrder(e.TreeOrder())
: nullptr;
StyleBuilder::ApplyProperty(property, state_,
ScopedCSSValue(*value, tree_scope));
StyleBuilder::ApplyProperty(property, state_, *value);
}
}
}
......@@ -540,11 +524,7 @@ void StyleCascade::LookupAndApplyDeclaration(const CSSProperty& property,
value = Resolve(property, *value, origin, resolver);
DCHECK(!value->IsVariableReferenceValue());
DCHECK(!value->IsPendingSubstitutionValue());
const TreeScope* tree_scope{nullptr};
if (origin == CascadeOrigin::kAuthor)
tree_scope = &TreeScopeAt(match_result_, priority.GetPosition());
StyleBuilder::ApplyProperty(property, state_,
ScopedCSSValue(*value, tree_scope));
StyleBuilder::ApplyProperty(property, state_, *value);
}
void StyleCascade::LookupAndApplyInterpolation(const CSSProperty& property,
......@@ -622,14 +602,11 @@ void StyleCascade::ForceColors() {
MaybeForceColor(GetCSSPropertyInternalVisitedTextEmphasisColor(),
style->InternalVisitedTextEmphasisColor());
ScopedCSSValue scoped_none(*CSSIdentifierValue::Create(CSSValueID::kNone),
nullptr);
StyleBuilder::ApplyProperty(GetCSSPropertyTextShadow(), state_, scoped_none);
StyleBuilder::ApplyProperty(GetCSSPropertyBoxShadow(), state_, scoped_none);
if (!style->HasUrlBackgroundImage()) {
StyleBuilder::ApplyProperty(GetCSSPropertyBackgroundImage(), state_,
scoped_none);
}
auto* none = CSSIdentifierValue::Create(CSSValueID::kNone);
StyleBuilder::ApplyProperty(GetCSSPropertyTextShadow(), state_, *none);
StyleBuilder::ApplyProperty(GetCSSPropertyBoxShadow(), state_, *none);
if (!style->HasUrlBackgroundImage())
StyleBuilder::ApplyProperty(GetCSSPropertyBackgroundImage(), state_, *none);
// Preserve the author/user defined background alpha channel.
style->SetBackgroundColor(
......@@ -651,9 +628,7 @@ void StyleCascade::MaybeForceColor(const CSSProperty& property,
return;
StyleBuilder::ApplyProperty(
property, state_,
ScopedCSSValue(*GetForcedColorValue(property.GetCSSPropertyName()),
nullptr));
property, state_, *GetForcedColorValue(property.GetCSSPropertyName()));
}
const CSSValue* StyleCascade::GetForcedColorValue(CSSPropertyName name) {
......
......@@ -125,12 +125,12 @@ class TestCascade {
}
void Apply(CascadeFilter filter = CascadeFilter()) {
EnsureAtLeast(CascadeOrigin::kAnimation);
EnsureAtLeast(CascadeOrigin::kAuthor);
cascade_.Apply(filter);
}
void ApplySingle(const CSSProperty& property) {
EnsureAtLeast(CascadeOrigin::kAnimation);
EnsureAtLeast(CascadeOrigin::kAuthor);
cascade_.AnalyzeIfNeeded();
TestCascadeResolver resolver(++cascade_.generation_);
cascade_.LookupAndApply(property, resolver.InnerResolver());
......@@ -234,12 +234,6 @@ class TestCascade {
current_origin_ = CascadeOrigin::kAuthor;
break;
case CascadeOrigin::kAuthor:
cascade_.MutableMatchResult().FinishAddingAuthorRulesForTreeScope(
GetDocument());
current_origin_ = CascadeOrigin::kAnimation;
break;
case CascadeOrigin::kAnimation:
break;
default:
NOTREACHED();
break;
......
......@@ -62,7 +62,6 @@
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_stats.h"
#include "third_party/blink/renderer/core/css/resolver/style_rule_usage_tracker.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/css/style_engine.h"
#include "third_party/blink/renderer/core/css/style_rule_import.h"
#include "third_party/blink/renderer/core/css/style_sheet_contents.h"
......@@ -1083,8 +1082,6 @@ CompositorKeyframeValue* StyleResolver::CreateCompositorKeyframeValueSnapshot(
cascade.MutableMatchResult().FinishAddingUARules();
cascade.MutableMatchResult().FinishAddingUserRules();
cascade.MutableMatchResult().AddMatchedProperties(set);
cascade.MutableMatchResult().FinishAddingAuthorRulesForTreeScope(
element.GetTreeScope());
cascade.Apply();
}
return CompositorKeyframeValueFactory::Create(property, *state.Style());
......@@ -1663,8 +1660,6 @@ const CSSValue* StyleResolver::ComputeValue(
cascade.MutableMatchResult().FinishAddingUARules();
cascade.MutableMatchResult().FinishAddingUserRules();
cascade.MutableMatchResult().AddMatchedProperties(set);
cascade.MutableMatchResult().FinishAddingAuthorRulesForTreeScope(
element->GetTreeScope());
cascade.Apply();
CSSPropertyRef property_ref(property_name, element->GetDocument());
......@@ -1790,14 +1785,9 @@ void StyleResolver::ComputeFont(Element& element,
for (const CSSProperty* property : properties) {
if (property->IDEquals(CSSPropertyID::kLineHeight))
UpdateFont(state);
// TODO(futhark): If we start supporting fonts on ShadowRoot.fonts in
// addition to Document.fonts, we need to pass the correct TreeScope instead
// of GetDocument() in the ScopedCSSValue below.
StyleBuilder::ApplyProperty(
*property, state,
ScopedCSSValue(
*property_set.GetPropertyCSSValue(property->PropertyID()),
&GetDocument()));
*property_set.GetPropertyCSSValue(property->PropertyID()));
}
}
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_SCOPED_CSS_VALUE_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_SCOPED_CSS_VALUE_H_
namespace blink {
class CSSValue;
class TreeScope;
// Store a CSSValue along with a TreeScope to support tree-scoped names and
// references for e.g. @font-face/font-family and @keyframes/animation-name.
// If the TreeScope pointer is null, we do not support such references, for
// instance for UA stylesheets.
class ScopedCSSValue {
STACK_ALLOCATED();
public:
ScopedCSSValue(const CSSValue& value, const TreeScope* tree_scope)
: value_(value), tree_scope_(tree_scope) {}
const CSSValue& GetCSSValue() const { return value_; }
const TreeScope* GetTreeScope() const { return tree_scope_; }
private:
const CSSValue& value_;
const TreeScope* tree_scope_;
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_CSS_SCOPED_CSS_VALUE_H_
......@@ -8,7 +8,6 @@
#include "third_party/blink/renderer/core/css/resolver/filter_operation_resolver.h"
#include "third_party/blink/renderer/core/css/resolver/style_builder.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/scoped_css_value.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/html/canvas/html_canvas_element.h"
#include "third_party/blink/renderer/core/paint/filter_effect_builder.h"
......@@ -366,9 +365,8 @@ sk_sp<PaintFilter> CanvasRenderingContext2DState::GetFilter(
filter_style.get(), filter_style.get());
resolver_state.SetStyle(filter_style);
StyleBuilder::ApplyProperty(
GetCSSPropertyFilter(), resolver_state,
ScopedCSSValue(*filter_value_, &style_resolution_host->GetDocument()));
StyleBuilder::ApplyProperty(GetCSSPropertyFilter(), resolver_state,
*filter_value_);
resolver_state.LoadPendingResources();
// We can't reuse m_fillFlags and m_strokeFlags for the filter, since these
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment