Skip to content

GitLab

T
tangled
Commit ff29b230 authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot
Browse files
Options

Make baseVal attribute synchronization explicit 

This moves the |base_value_updated_| flag from the "primitive type"
version of SVGAnimatedProperty to SVGAnimatedPropertyBase, and sets it
in BaseValueChanged. The flag is cleared in SynchronizeAttribute.
This allows some code to be simplified. It also avoids synchronizations
triggered by just having created the tear-off object like in the bug.

Bug: 873470
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I59f30dd69286dc0799a583eb6a52c41022b0af91
Reviewed-on: https://chromium-review.googlesource.com/1188305
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585852}
parent ff1335d9
Hide whitespace changes
Inline Side-by-side
Showing with 39 additions and 42 deletions
third_party/WebKit/LayoutTests/svg/dom/no-value-synching-after-property-get.html 0 → 100644
View file @ ff29b230
<!DOCTYPE html>
<title>Getting a property should not update the corresponding attribute</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<svg>
<rect width="100" height="50" fill="green" transform="scale(1)"/>
</svg>
<script>
test(function() {
var rect = document.querySelector('rect');
var clone = rect.cloneNode(true);
clone.systemLanguage;
clone.transform.baseVal.getItem(0).setTranslate(0, 50);
rect.parentNode.appendChild(clone);
assert_false(clone.hasAttribute('systemLanguage'));
});
</script>
third_party/blink/renderer/core/svg/properties/svg_animated_property.cc
View file @ ff29b230
......@@ -44,6 +44,7 @@ SVGAnimatedPropertyBase::SVGAnimatedPropertyBase(
// property enum. CSS properties that don't fit in this bitfield are never
// used here. See static_assert in header.
css_property_id_(static_cast<unsigned>(css_property_id)),
base_value_needs_synchronization_(false),
context_element_(context_element),
attribute_name_(attribute_name) {
DCHECK(context_element_);
......@@ -62,14 +63,24 @@ void SVGAnimatedPropertyBase::AnimationEnded() {
SynchronizeAttribute();
}
bool SVGAnimatedPropertyBase::NeedsSynchronizeAttribute() const {
// DOM attribute synchronization is only needed if a change has been made
// through JavaScript (via a tear-off or primitive) or the property is being
// animated. This prevents unnecessary attribute creation on the target
// element.
return base_value_needs_synchronization_ || IsAnimating();
}
void SVGAnimatedPropertyBase::SynchronizeAttribute() {
AtomicString value(CurrentValueBase()->ValueAsString());
context_element_->SetSynchronizedLazyAttribute(attribute_name_, value);
base_value_needs_synchronization_ = false;
}
void SVGAnimatedPropertyBase::BaseValueChanged() {
DCHECK(context_element_);
DCHECK(attribute_name_ != QualifiedName::Null());
base_value_needs_synchronization_ = true;
context_element_->InvalidateSVGAttributes();
context_element_->SvgAttributeBaseValChanged(attribute_name_);
}
......
third_party/blink/renderer/core/svg/properties/svg_animated_property.h
View file @ ff29b230
......@@ -58,7 +58,7 @@ class SVGAnimatedPropertyBase : public GarbageCollectedMixin {
virtual void AnimationEnded();
virtual SVGParsingError AttributeChanged(const String&) = 0;
virtual bool NeedsSynchronizeAttribute() = 0;
virtual bool NeedsSynchronizeAttribute() const;
virtual void SynchronizeAttribute();
AnimatedPropertyType GetType() const {
......@@ -99,6 +99,7 @@ class SVGAnimatedPropertyBase : public GarbageCollectedMixin {
const unsigned type_ : 5;
const unsigned css_property_id_ : kCssPropertyBits;
unsigned base_value_needs_synchronization_ : 1;
TraceWrapperMember<SVGElement> context_element_;
const QualifiedName& attribute_name_;
DISALLOW_COPY_AND_ASSIGN(SVGAnimatedPropertyBase);
......@@ -172,18 +173,6 @@ template <typename Property,
typename PrimitiveType = typename Property::PrimitiveType>
class SVGAnimatedProperty : public SVGAnimatedPropertyCommon<Property> {
public:
bool NeedsSynchronizeAttribute() override {
// DOM attribute synchronization is only needed if tear-off is being touched
// from javascript or the property is being animated. This prevents
// unnecessary attribute creation on target element.
return base_value_updated_ || this->IsAnimating();
}
void SynchronizeAttribute() override {
SVGAnimatedPropertyBase::SynchronizeAttribute();
base_value_updated_ = false;
}
// SVGAnimated* DOM Spec implementations:
// baseVal()/setBaseVal()/animVal() are only to be used from SVG DOM
......@@ -192,7 +181,6 @@ class SVGAnimatedProperty : public SVGAnimatedPropertyCommon<Property> {
void setBaseVal(PrimitiveType value, ExceptionState&) {
this->BaseValue()->SetValue(value);
base_value_updated_ = true;
this->BaseValueChanged();
}
......@@ -209,10 +197,7 @@ class SVGAnimatedProperty : public SVGAnimatedPropertyCommon<Property> {
: SVGAnimatedPropertyCommon<Property>(context_element,
attribute_name,
initial_value,
css_property_id),
base_value_updated_(false) {}
bool base_value_updated_;
css_property_id) {}
};
// Implementation of SVGAnimatedProperty which uses tear-off value types.
......@@ -242,13 +227,6 @@ class SVGAnimatedProperty<Property, TearOffType, void>
UpdateAnimValTearOffIfNeeded();
}
bool NeedsSynchronizeAttribute() override {
// DOM attribute synchronization is only needed if tear-off is being touched
// from javascript or the property is being animated. This prevents
// unnecessary attribute creation on target element.
return base_val_tear_off_ || this->IsAnimating();
}
// SVGAnimated* DOM Spec implementations:
// baseVal()/animVal() are only to be used from SVG DOM implementation.
......@@ -316,12 +294,6 @@ class SVGAnimatedProperty<Property, void, void>
initial_value, css_property_id);
}
bool NeedsSynchronizeAttribute() override {
// DOM attribute synchronization is only needed if the property is being
// animated.
return this->IsAnimating();
}
protected:
SVGAnimatedProperty(SVGElement* context_element,
const QualifiedName& attribute_name,
......
third_party/blink/renderer/core/svg/svg_animated_angle.cc
View file @ ff29b230
......@@ -51,7 +51,7 @@ void SVGAnimatedAngle::Trace(blink::Visitor* visitor) {
ScriptWrappable::Trace(visitor);
}
bool SVGAnimatedAngle::NeedsSynchronizeAttribute() {
bool SVGAnimatedAngle::NeedsSynchronizeAttribute() const {
return orient_type_->NeedsSynchronizeAttribute() ||
SVGAnimatedProperty<SVGAngle>::NeedsSynchronizeAttribute();
}
......
third_party/blink/renderer/core/svg/svg_animated_angle.h
View file @ ff29b230
......@@ -55,7 +55,7 @@ class SVGAnimatedAngle final : public ScriptWrappable,
}
// SVGAnimatedPropertyBase:
bool NeedsSynchronizeAttribute() override;
bool NeedsSynchronizeAttribute() const override;
void SynchronizeAttribute() override;
void SetAnimatedValue(SVGPropertyBase*) override;
......
third_party/blink/renderer/core/svg/svg_animated_integer_optional_integer.cc
View file @ ff29b230
......@@ -73,7 +73,7 @@ void SVGAnimatedIntegerOptionalInteger::AnimationEnded() {
second_integer_->AnimationEnded();
}
bool SVGAnimatedIntegerOptionalInteger::NeedsSynchronizeAttribute() {
bool SVGAnimatedIntegerOptionalInteger::NeedsSynchronizeAttribute() const {
return first_integer_->NeedsSynchronizeAttribute() ||
second_integer_->NeedsSynchronizeAttribute();
}
......
third_party/blink/renderer/core/svg/svg_animated_integer_optional_integer.h
View file @ ff29b230
......@@ -60,7 +60,7 @@ class SVGAnimatedIntegerOptionalInteger
}
void SetAnimatedValue(SVGPropertyBase*) override;
bool NeedsSynchronizeAttribute() override;
bool NeedsSynchronizeAttribute() const override;
void AnimationEnded() override;
SVGAnimatedInteger* FirstInteger() { return first_integer_.Get(); }
......
third_party/blink/renderer/core/svg/svg_animated_number_optional_number.cc
View file @ ff29b230
......@@ -60,7 +60,7 @@ void SVGAnimatedNumberOptionalNumber::AnimationEnded() {
second_number_->AnimationEnded();
}
bool SVGAnimatedNumberOptionalNumber::NeedsSynchronizeAttribute() {
bool SVGAnimatedNumberOptionalNumber::NeedsSynchronizeAttribute() const {
return first_number_->NeedsSynchronizeAttribute() ||
second_number_->NeedsSynchronizeAttribute();
}
......
third_party/blink/renderer/core/svg/svg_animated_number_optional_number.h
View file @ ff29b230
......@@ -60,7 +60,7 @@ class SVGAnimatedNumberOptionalNumber
}
void SetAnimatedValue(SVGPropertyBase*) override;
bool NeedsSynchronizeAttribute() override;
bool NeedsSynchronizeAttribute() const override;
void AnimationEnded() override;
SVGAnimatedNumber* FirstNumber() { return first_number_.Get(); }
......
third_party/blink/renderer/core/svg/svg_static_string_list.cc
View file @ ff29b230
......@@ -77,10 +77,6 @@ void SVGStaticStringList::AnimationEnded() {
NOTREACHED();
}
bool SVGStaticStringList::NeedsSynchronizeAttribute() {
return tear_off_;
}
SVGStringListTearOff* SVGStaticStringList::TearOff() {
if (!tear_off_) {
tear_off_ =
......
third_party/blink/renderer/core/svg/svg_static_string_list.h
View file @ ff29b230
......@@ -62,7 +62,6 @@ class SVGStaticStringList final
SVGPropertyBase* CreateAnimatedValue() override;
void SetAnimatedValue(SVGPropertyBase*) override;
void AnimationEnded() override;
bool NeedsSynchronizeAttribute() override;
SVGParsingError AttributeChanged(const String&) override;
......
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