Commit 9c9a1794 authored by Oriol Brufau's avatar Oriol Brufau Committed by Chromium LUCI CQ

[css-logical] Fix perf regression in cssom "set a CSS declaration"

r834202 changed MutableCSSPropertyValueSet::SetProperty so that, when
changing the value of a property that belongs to a logical property
group, which is followed by a property in the same logical property
group but opposite mapping logic, the changed property is moved to the
end, to ensure it takes precedence.

However, this regressed the CSSPropertyUpdateValue perf test.

This patch adds a may_have_logical_properties_ flag. When false, it
means the MutableCSSPropertyValueSet has no logical property, so we can
skip the extra work added in r834202.

Bug: 1156321
Change-Id: I4ab6d92cf51126bcab16ffa9635bf91c98d6f738
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640097Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#846231}
parent e96b92d4
......@@ -81,8 +81,14 @@ MutableCSSPropertyValueSet::MutableCSSPropertyValueSet(
unsigned length)
: CSSPropertyValueSet(kHTMLStandardMode) {
property_vector_.ReserveInitialCapacity(length);
for (unsigned i = 0; i < length; ++i)
for (unsigned i = 0; i < length; ++i) {
property_vector_.UncheckedAppend(properties[i]);
if (!may_have_logical_properties_) {
const CSSProperty& prop = CSSProperty::Get(properties[i].Id());
may_have_logical_properties_ =
prop.IsInLogicalPropertyGroup() && prop.IsSurrogate();
}
}
}
ImmutableCSSPropertyValueSet::ImmutableCSSPropertyValueSet(
......@@ -180,12 +186,19 @@ MutableCSSPropertyValueSet::MutableCSSPropertyValueSet(
if (auto* other_mutable_property_set =
DynamicTo<MutableCSSPropertyValueSet>(other)) {
property_vector_ = other_mutable_property_set->property_vector_;
may_have_logical_properties_ =
other_mutable_property_set->may_have_logical_properties_;
} else {
property_vector_.ReserveInitialCapacity(other.PropertyCount());
for (unsigned i = 0; i < other.PropertyCount(); ++i) {
PropertyReference property = other.PropertyAt(i);
property_vector_.UncheckedAppend(
CSSPropertyValue(property.PropertyMetadata(), property.Value()));
if (!may_have_logical_properties_) {
const CSSProperty& prop = CSSProperty::Get(property.Id());
may_have_logical_properties_ =
prop.IsInLogicalPropertyGroup() && prop.IsSurrogate();
}
}
}
}
......@@ -418,16 +431,18 @@ bool MutableCSSPropertyValueSet::SetProperty(const CSSPropertyValue& property,
CSSPropertyValue* to_replace =
slot ? slot : FindCSSPropertyWithName(property.Name());
if (to_replace) {
const CSSProperty& prop = CSSProperty::Get(property.Id());
if (prop.IsInLogicalPropertyGroup()) {
DCHECK(property_vector_.Contains(*to_replace));
int to_replace_index = to_replace - property_vector_.begin();
for (int n = property_vector_.size() - 1; n > to_replace_index; --n) {
if (prop.IsInSameLogicalPropertyGroupWithDifferentMappingLogic(
PropertyAt(n).Id())) {
RemovePropertyAtIndex(to_replace_index, nullptr);
to_replace = nullptr;
break;
if (may_have_logical_properties_) {
const CSSProperty& prop = CSSProperty::Get(property.Id());
if (prop.IsInLogicalPropertyGroup()) {
DCHECK(property_vector_.Contains(*to_replace));
int to_replace_index = to_replace - property_vector_.begin();
for (int n = property_vector_.size() - 1; n > to_replace_index; --n) {
if (prop.IsInSameLogicalPropertyGroupWithDifferentMappingLogic(
PropertyAt(n).Id())) {
RemovePropertyAtIndex(to_replace_index, nullptr);
to_replace = nullptr;
break;
}
}
}
}
......@@ -437,6 +452,10 @@ bool MutableCSSPropertyValueSet::SetProperty(const CSSPropertyValue& property,
*to_replace = property;
return true;
}
} else if (!may_have_logical_properties_) {
const CSSProperty& prop = CSSProperty::Get(property.Id());
may_have_logical_properties_ =
prop.IsInLogicalPropertyGroup() && prop.IsSurrogate();
}
property_vector_.push_back(property);
return true;
......@@ -518,6 +537,7 @@ bool CSSPropertyValueSet::HasFailedOrCanceledSubresources() const {
void MutableCSSPropertyValueSet::Clear() {
property_vector_.clear();
may_have_logical_properties_ = false;
}
inline bool ContainsId(const CSSProperty* const set[],
......
......@@ -298,6 +298,7 @@ class CORE_EXPORT MutableCSSPropertyValueSet : public CSSPropertyValueSet {
friend class CSSPropertyValueSet;
HeapVector<CSSPropertyValue, 4> property_vector_;
bool may_have_logical_properties_{false};
};
template <>
......
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