Synchronize classList on SVGElement.className.baseVal updates

Element.classList is normally updated by AttributeChanged(). When the
underlying attribute is updated via the corresponding SVG animated
property AttributeChanged() is not invoked, which makes the list and
attribute go out of sync.

Refactor baseVal updates on SVGElement so that they go through a single
entry point by renaming SvgAttributeBaseValChanged() to
BaseValueChanged() and folding it into the other user
(SVGElement::AttributeChanged). Fold InvalidateSVGAttributes() into
BaseValueChanged() since it's the only caller.

BaseValueChanged() is then updated to update the classList when the
className base value is updated. Move the classList update into a shared
Element::UpdateClassList() function.

Fixed: 1137953
Change-Id: I64efff0bfd163b14eb937c635c3d170b4511f4a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480023
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818035}

third_party/blink/renderer/core/dom/element.cc

@@ -2195,10 +2195,7 @@ void Element::AttributeChanged(const AttributeModificationParams& params) {
     }
   } else if (name == html_names::kClassAttr) {
     ClassAttributeChanged(params.new_value);
-    if (HasRareData() && GetElementRareData()->GetClassList()) {
-      GetElementRareData()->GetClassList()->DidUpdateAttributeValue(
-          params.old_value, params.new_value);
-    }
+    UpdateClassList(params.old_value, params.new_value);
   } else if (name == html_names::kNameAttr) {
     SetHasName(!params.new_value.IsNull());
   } else if (name == html_names::kPartAttr) {
@@ -2302,6 +2299,14 @@ void Element::ClassAttributeChanged(const AtomicString& new_class_string) {
   }
 }
 
+void Element::UpdateClassList(const AtomicString& old_class_string,
+                              const AtomicString& new_class_string) {
+  if (!HasRareData())
+    return;
+  if (DOMTokenList* class_list = GetElementRareData()->GetClassList())
+    class_list->DidUpdateAttributeValue(old_class_string, new_class_string);
+}
+
 bool Element::ShouldInvalidateDistributionWhenAttributeChanged(
     ShadowRoot& shadow_root,
     const QualifiedName& name,

third_party/blink/renderer/core/dom/element.h

@@ -975,10 +975,13 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
   // create layout objects is completed (e.g. in display-locked trees).
   bool IsFocusableStyleAfterUpdate() const;
 
-  // classAttributeChanged() exists to share code between
-  // parseAttribute (called via setAttribute()) and
-  // svgAttributeChanged (called when element.className.baseValue is set)
+  // ClassAttributeChanged() and UpdateClassList() exist to share code between
+  // ParseAttribute (called via setAttribute()) and SvgAttributeChanged (called
+  // when element.className.baseVal is set or when the 'class' attribute is
+  // animated by SMIL).
   void ClassAttributeChanged(const AtomicString& new_class_string);
+  void UpdateClassList(const AtomicString& old_class_string,
+                       const AtomicString& new_class_string);
 
   static bool AttributeValueIsJavaScriptURL(const Attribute&);
 

third_party/blink/renderer/core/svg/properties/svg_animated_property.cc

@@ -79,8 +79,7 @@ void SVGAnimatedPropertyBase::BaseValueChanged() {
   DCHECK(context_element_);
   DCHECK(attribute_name_ != QualifiedName::Null());
   base_value_needs_synchronization_ = true;
-  context_element_->InvalidateSVGAttributes();
-  context_element_->SvgAttributeBaseValChanged(attribute_name_);
+  context_element_->BaseValueChanged(*this);
 }
 
 void SVGAnimatedPropertyBase::EnsureAnimValUpdated() {

third_party/blink/renderer/core/svg/svg_element.cc

@@ -926,7 +926,8 @@ void SVGElement::AttributeChanged(const AttributeModificationParams& params) {
   if (params.name == html_names::kStyleAttr)
     return;
 
-  SvgAttributeBaseValChanged(params.name);
+  SvgAttributeChanged(params.name);
+  UpdateWebAnimatedAttributeOnBaseValChange(params.name);
 }
 
 void SVGElement::SvgAttributeChanged(const QualifiedName& attr_name) {
@@ -944,8 +945,15 @@ void SVGElement::SvgAttributeChanged(const QualifiedName& attr_name) {
   }
 }
 
-void SVGElement::SvgAttributeBaseValChanged(const QualifiedName& attribute) {
+void SVGElement::BaseValueChanged(
+    const SVGAnimatedPropertyBase& animated_property) {
+  const QualifiedName& attribute = animated_property.AttributeName();
+  EnsureUniqueElementData().SetSvgAttributesAreDirty(true);
   SvgAttributeChanged(attribute);
+  if (class_name_ == &animated_property) {
+    UpdateClassList(g_null_atom,
+                    AtomicString(class_name_->BaseValue()->Value()));
+  }
   UpdateWebAnimatedAttributeOnBaseValChange(attribute);
 }
 

third_party/blink/renderer/core/svg/svg_element.h

@@ -93,6 +93,7 @@ class CORE_EXPORT SVGElement : public Element {
   void SetWebAnimationsPending();
   void ApplyActiveWebAnimations();
 
+  void BaseValueChanged(const SVGAnimatedPropertyBase&);
   void EnsureAttributeAnimValUpdated();
 
   void SetWebAnimatedAttribute(const QualifiedName& attribute,
@@ -122,7 +123,6 @@ class CORE_EXPORT SVGElement : public Element {
   virtual bool IsValid() const { return true; }
 
   virtual void SvgAttributeChanged(const QualifiedName&);
-  void SvgAttributeBaseValChanged(const QualifiedName&);
 
   SVGAnimatedPropertyBase* PropertyFromAttribute(
       const QualifiedName& attribute_name) const;
@@ -134,9 +134,6 @@ class CORE_EXPORT SVGElement : public Element {
 
   virtual AffineTransform* AnimateMotionTransform() { return nullptr; }
 
-  void InvalidateSVGAttributes() {
-    EnsureUniqueElementData().SetSvgAttributesAreDirty(true);
-  }
   void InvalidateSVGPresentationAttributeStyle() {
     EnsureUniqueElementData().SetPresentationAttributeStyleIsDirty(true);
   }

third_party/blink/web_tests/external/wpt/svg/types/scripted/SVGElement.className-01.svg

+<svg xmlns="http://www.w3.org/2000/svg" xmlns:h="http://www.w3.org/1999/xhtml">
+  <title>SVGElement.prototype.className: Reflects to .classList</title>
+  <h:link rel="help" href="https://svgwg.org/svg2-draft/types.html#__svg__SVGElement__className"/>
+  <h:script src="/resources/testharness.js"/>
+  <h:script src="/resources/testharnessreport.js"/>
+  <script>
+    test(() => {
+      const element = document.createElementNS('http://www.w3.org/2000/svg', 'g');
+
+      assert_false(element.classList.contains('one'), "classList is initially empty");
+      element.className.baseVal = 'one';
+      assert_true(element.classList.contains('one'), "className should reflect to classList");
+    });
+</script>
+</svg>