Refactor SVG*List types that store SVGProperty objects

Introduce a SVGListPropertyBase class that SVG*List types can inherit
from to get generic list handling. This deduplicates code that will be
essentially identical already. The type-specific SVG*List class now only
provides type-specific access to the underlying list.

This also allows more straight-forward sharing of for example the
ValueAsString() implementation,(now depending on the virtual method on
the listed object wrapped in generic list-string building.

The interface exposed to the DOM environment is moved to the
corresponding tear-off classes, and the interface of the list base class
is simplified to a slightly smaller set of operations.

Split SVGListPropertyTearOffHelper::CreateItemTearOff into variants for
"attached to list" and "detached from list" since we know this at the
callsites.

Bug: 1106762
Change-Id: I8263bb957ed4824ad70341fd4dd161d1214e697e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302614
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799756}

third_party/blink/renderer/core/fullscreen/fullscreen.cc

@@ -54,6 +54,7 @@
 #include "third_party/blink/renderer/core/page/chrome_client.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/svg/svg_svg_element.h"
+#include "third_party/blink/renderer/platform/bindings/exception_messages.h"
 #include "third_party/blink/renderer/platform/bindings/microtask.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/instrumentation/use_counter.h"

third_party/blink/renderer/core/svg/BUILD.gn

@@ -35,6 +35,8 @@ blink_core_sources("svg") {
     "pattern_attributes.h",
     "properties/svg_animated_property.cc",
     "properties/svg_animated_property.h",
+    "properties/svg_list_property.cc",
+    "properties/svg_list_property.h",
     "properties/svg_list_property_helper.h",
     "properties/svg_list_property_tear_off_helper.h",
     "properties/svg_property.h",

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

+/*
+ * Copyright (C) 2013 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "third_party/blink/renderer/core/svg/properties/svg_list_property.h"
+
+#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
+
+namespace blink {
+
+void SVGListPropertyBase::Clear() {
+  // Detach all list items as they are no longer part of this list.
+  for (auto& value : values_) {
+    DCHECK_EQ(value->OwnerList(), this);
+    value->SetOwnerList(nullptr);
+  }
+  values_.clear();
+}
+
+void SVGListPropertyBase::Insert(uint32_t index, SVGPropertyBase* new_item) {
+  values_.insert(index, new_item);
+  new_item->SetOwnerList(this);
+}
+
+void SVGListPropertyBase::Remove(uint32_t index) {
+  DCHECK_EQ(values_[index]->OwnerList(), this);
+  values_[index]->SetOwnerList(nullptr);
+  values_.EraseAt(index);
+}
+
+void SVGListPropertyBase::Append(SVGPropertyBase* new_item) {
+  values_.push_back(new_item);
+  new_item->SetOwnerList(this);
+}
+
+void SVGListPropertyBase::Replace(uint32_t index, SVGPropertyBase* new_item) {
+  DCHECK_EQ(values_[index]->OwnerList(), this);
+  values_[index]->SetOwnerList(nullptr);
+  values_[index] = new_item;
+  new_item->SetOwnerList(this);
+}
+
+String SVGListPropertyBase::ValueAsString() const {
+  if (values_.IsEmpty())
+    return String();
+
+  StringBuilder builder;
+
+  auto* it = values_.begin();
+  auto* it_end = values_.end();
+  while (it != it_end) {
+    builder.Append((*it)->ValueAsString());
+    ++it;
+    if (it != it_end)
+      builder.Append(' ');
+  }
+  return builder.ToString();
+}
+
+}  // namespace blink

third_party/blink/renderer/core/svg/properties/svg_list_property.h

+/*
+ * Copyright (C) 2013 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_SVG_PROPERTIES_SVG_LIST_PROPERTY_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_SVG_PROPERTIES_SVG_LIST_PROPERTY_H_
+
+#include "third_party/blink/renderer/core/svg/properties/svg_property.h"
+#include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
+#include "third_party/blink/renderer/platform/wtf/vector.h"
+
+namespace blink {
+
+// Base class for properties that represent lists of other properties. Used to
+// implement SVG*List types that hold objects (SVGLengthList, SVGNumberList,
+// SVGPointList and SVGTransformList).
+class SVGListPropertyBase : public SVGPropertyBase {
+ protected:
+  using ListType = HeapVector<Member<SVGPropertyBase>>;
+
+  uint32_t length() const { return values_.size(); }
+  bool IsEmpty() const { return !length(); }
+
+  using const_iterator = typename ListType::const_iterator;
+  const_iterator begin() const { return values_.begin(); }
+  const_iterator end() const { return values_.end(); }
+
+  SVGPropertyBase* at(uint32_t index) {
+    DCHECK_LT(index, values_.size());
+    DCHECK_EQ(values_[index]->OwnerList(), this);
+    return values_[index].Get();
+  }
+
+  const SVGPropertyBase* at(uint32_t index) const {
+    return const_cast<SVGListPropertyBase*>(this)->at(index);
+  }
+
+  void Clear();
+  void Insert(uint32_t index, SVGPropertyBase* new_item);
+  void Remove(uint32_t index);
+  void Append(SVGPropertyBase* new_item);
+  void Replace(uint32_t index, SVGPropertyBase* new_item);
+
+ public:
+  String ValueAsString() const final;
+
+  void Trace(Visitor* visitor) const final {
+    visitor->Trace(values_);
+    SVGPropertyBase::Trace(visitor);
+  }
+
+ private:
+  ListType values_;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_PROPERTIES_SVG_LIST_PROPERTY_H_

third_party/blink/renderer/core/svg/properties/svg_list_property_helper.h

@@ -31,56 +31,74 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_SVG_PROPERTIES_SVG_LIST_PROPERTY_HELPER_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_SVG_PROPERTIES_SVG_LIST_PROPERTY_HELPER_H_
 
-#include "third_party/blink/renderer/core/svg/properties/svg_property_helper.h"
-#include "third_party/blink/renderer/platform/bindings/exception_messages.h"
-#include "third_party/blink/renderer/platform/bindings/exception_state.h"
-#include "third_party/blink/renderer/platform/heap/heap.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
-#include "third_party/blink/renderer/platform/wtf/vector.h"
+#include "third_party/blink/renderer/core/svg/properties/svg_list_property.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace blink {
 
-// This is an implementation of the SVG*List property spec:
-// http://www.w3.org/TR/SVG/single-page.html#types-InterfaceSVGLengthList
+// Typed wrapper for SVG*List properties that adds type-dependent operations.
 template <typename Derived, typename ItemProperty>
-class SVGListPropertyHelper : public SVGPropertyHelper<Derived> {
+class SVGListPropertyHelper : public SVGListPropertyBase {
  public:
   typedef ItemProperty ItemPropertyType;
-  using ItemPropertyListType = HeapVector<Member<ItemPropertyType>>;
 
   SVGListPropertyHelper() = default;
-
   ~SVGListPropertyHelper() override = default;
 
-  // used from Blink C++ code:
+  class const_iterator {
+   public:
+    explicit const_iterator(SVGListPropertyBase::const_iterator wrapped)
+        : wrapped_(wrapped) {}
+
+    const_iterator& operator++() {
+      ++wrapped_;
+      return *this;
+    }
+    bool operator==(const const_iterator& other) const {
+      return wrapped_ == other.wrapped_;
+    }
+    bool operator!=(const const_iterator& other) const {
+      return !operator==(other);
+    }
+    const ItemPropertyType* operator->() const {
+      return To<ItemPropertyType>(wrapped_->Get());
+    }
+    const ItemPropertyType* operator*() const {
+      return To<ItemPropertyType>(wrapped_->Get());
+    }
+
+   private:
+    SVGListPropertyBase::const_iterator wrapped_;
+  };
+  const_iterator begin() const {
+    return const_iterator(SVGListPropertyBase::begin());
+  }
+  const_iterator end() const {
+    return const_iterator(SVGListPropertyBase::end());
+  }
+
+  using SVGListPropertyBase::IsEmpty;
+  using SVGListPropertyBase::length;
 
   ItemPropertyType* at(uint32_t index) {
-    DCHECK_LT(index, values_.size());
-    DCHECK_EQ(values_.at(index)->OwnerList(), this);
-    return values_.at(index).Get();
+    return To<ItemPropertyType>(SVGListPropertyBase::at(index));
   }
 
   const ItemPropertyType* at(uint32_t index) const {
-    return const_cast<SVGListPropertyHelper<Derived, ItemProperty>*>(this)->at(
-        index);
+    return To<ItemPropertyType>(SVGListPropertyBase::at(index));
   }
 
-  using const_iterator = typename ItemPropertyListType::const_iterator;
-  const_iterator begin() const { return values_.begin(); }
-  const_iterator end() const { return values_.end(); }
-
+  using SVGListPropertyBase::Clear;
+  void Insert(uint32_t index, ItemPropertyType* new_item) {
+    SVGListPropertyBase::Insert(index, new_item);
+  }
+  using SVGListPropertyBase::Remove;
   void Append(ItemPropertyType* new_item) {
-    DCHECK(new_item);
-    values_.push_back(new_item);
-    new_item->SetOwnerList(this);
+    SVGListPropertyBase::Append(new_item);
   }
-
-  bool operator==(const Derived& other) const {
-    return values_ == other.values_;
+  void Replace(uint32_t index, ItemPropertyType* new_item) {
+    SVGListPropertyBase::Replace(index, new_item);
   }
-  bool operator!=(const Derived& other) const { return !(*this == other); }
-
-  bool IsEmpty() const { return !length(); }
 
   virtual Derived* Clone() {
     auto* svg_list = MakeGarbageCollected<Derived>();
@@ -88,24 +106,14 @@ class SVGListPropertyHelper : public SVGPropertyHelper<Derived> {
     return svg_list;
   }
 
-  // SVGList*Property DOM spec:
-
-  uint32_t length() const { return values_.size(); }
-
-  void Clear();
-
-  ItemPropertyType* Initialize(ItemPropertyType*);
-  ItemPropertyType* GetItem(uint32_t, ExceptionState&);
-  ItemPropertyType* InsertItemBefore(ItemPropertyType*, uint32_t);
-  ItemPropertyType* RemoveItem(uint32_t, ExceptionState&);
-  ItemPropertyType* AppendItem(ItemPropertyType*);
-  ItemPropertyType* ReplaceItem(ItemPropertyType*, uint32_t, ExceptionState&);
-
-  void Trace(Visitor* visitor) const override {
-    visitor->Trace(values_);
-    SVGPropertyHelper<Derived>::Trace(visitor);
+  SVGPropertyBase* CloneForAnimation(const String& value) const override {
+    auto* property = MakeGarbageCollected<Derived>();
+    property->SetValueAsString(value);
+    return property;
   }
 
+  AnimatedPropertyType GetType() const override { return Derived::ClassType(); }
+
  protected:
   void DeepCopy(const Derived*);
 
@@ -114,123 +122,16 @@ class SVGListPropertyHelper : public SVGPropertyHelper<Derived> {
                               float percentage,
                               bool is_to_animation);
 
-  String SerializeList() const;
-
   virtual ItemPropertyType* CreatePaddingItem() const {
     return MakeGarbageCollected<ItemPropertyType>();
   }
-
- private:
-  inline bool CheckIndexBound(uint32_t, ExceptionState&);
-
-  ItemPropertyListType values_;
 };
 
-template <typename Derived, typename ItemProperty>
-void SVGListPropertyHelper<Derived, ItemProperty>::Clear() {
-  // Detach all list items as they are no longer part of this list.
-  for (auto& value : values_) {
-    DCHECK_EQ(value->OwnerList(), this);
-    value->SetOwnerList(nullptr);
-  }
-  values_.clear();
-}
-
-template <typename Derived, typename ItemProperty>
-ItemProperty* SVGListPropertyHelper<Derived, ItemProperty>::Initialize(
-    ItemProperty* new_item) {
-  // Spec: Clears all existing current items from the list and re-initializes
-  // the list to hold the single item specified by the parameter.
-  Clear();
-  Append(new_item);
-  return new_item;
-}
-
-template <typename Derived, typename ItemProperty>
-ItemProperty* SVGListPropertyHelper<Derived, ItemProperty>::GetItem(
-    uint32_t index,
-    ExceptionState& exception_state) {
-  if (!CheckIndexBound(index, exception_state))
-    return nullptr;
-  return at(index);
-}
-
-template <typename Derived, typename ItemProperty>
-ItemProperty* SVGListPropertyHelper<Derived, ItemProperty>::InsertItemBefore(
-    ItemProperty* new_item,
-    uint32_t index) {
-  // Spec: If the index is greater than or equal to length, then the new item is
-  // appended to the end of the list.
-  if (index > values_.size())
-    index = values_.size();
-
-  // Spec: Inserts a new item into the list at the specified position. The index
-  // of the item before which the new item is to be inserted. The first item is
-  // number 0. If the index is equal to 0, then the new item is inserted at the
-  // front of the list.
-  values_.insert(index, new_item);
-  new_item->SetOwnerList(this);
-  return new_item;
-}
-
-template <typename Derived, typename ItemProperty>
-ItemProperty* SVGListPropertyHelper<Derived, ItemProperty>::RemoveItem(
-    uint32_t index,
-    ExceptionState& exception_state) {
-  if (!CheckIndexBound(index, exception_state))
-    return nullptr;
-
-  DCHECK_EQ(values_.at(index)->OwnerList(), this);
-  ItemPropertyType* old_item = values_.at(index);
-  values_.EraseAt(index);
-  old_item->SetOwnerList(nullptr);
-  return old_item;
-}
-
-template <typename Derived, typename ItemProperty>
-ItemProperty* SVGListPropertyHelper<Derived, ItemProperty>::AppendItem(
-    ItemProperty* new_item) {
-  // Append the value and wrapper at the end of the list.
-  Append(new_item);
-  return new_item;
-}
-
-template <typename Derived, typename ItemProperty>
-ItemProperty* SVGListPropertyHelper<Derived, ItemProperty>::ReplaceItem(
-    ItemProperty* new_item,
-    uint32_t index,
-    ExceptionState& exception_state) {
-  if (!CheckIndexBound(index, exception_state))
-    return nullptr;
-
-  // Update the value at the desired position 'index'.
-  Member<ItemPropertyType>& position = values_[index];
-  DCHECK_EQ(position->OwnerList(), this);
-  position->SetOwnerList(nullptr);
-  position = new_item;
-  new_item->SetOwnerList(this);
-  return new_item;
-}
-
-template <typename Derived, typename ItemProperty>
-bool SVGListPropertyHelper<Derived, ItemProperty>::CheckIndexBound(
-    uint32_t index,
-    ExceptionState& exception_state) {
-  if (index >= values_.size()) {
-    exception_state.ThrowDOMException(
-        DOMExceptionCode::kIndexSizeError,
-        ExceptionMessages::IndexExceedsMaximumBound("index", index,
-                                                    values_.size()));
-    return false;
-  }
-  return true;
-}
-
 template <typename Derived, typename ItemProperty>
 void SVGListPropertyHelper<Derived, ItemProperty>::DeepCopy(
     const Derived* from) {
   Clear();
-  for (const auto& from_value : from->values_)
+  for (const auto* from_value : *from)
     Append(from_value->Clone());
 }
 
@@ -265,24 +166,6 @@ bool SVGListPropertyHelper<Derived, ItemProperty>::AdjustFromToListValues(
   return true;
 }
 
-template <typename Derived, typename ItemProperty>
-String SVGListPropertyHelper<Derived, ItemProperty>::SerializeList() const {
-  if (values_.IsEmpty())
-    return String();
-
-  StringBuilder builder;
-
-  auto it = values_.begin();
-  auto it_end = values_.end();
-  while (it != it_end) {
-    builder.Append((*it)->ValueAsString());
-    ++it;
-    if (it != it_end)
-      builder.Append(' ');
-  }
-  return builder.ToString();
-}
-
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_PROPERTIES_SVG_LIST_PROPERTY_HELPER_H_

third_party/blink/renderer/core/svg/properties/svg_list_property_tear_off_helper.h

@@ -38,6 +38,8 @@
 
 namespace blink {
 
+// This is an implementation of the SVG*List property spec:
+// https://svgwg.org/svg2-draft/types.html#ListInterfaces
 template <typename Derived, typename ListProperty>
 class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
  public:
@@ -66,16 +68,25 @@ class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
       return nullptr;
     }
     DCHECK(item);
-    ItemPropertyType* value = ToDerived()->Target()->Initialize(
-        GetValueForInsertionFromTearOff(item));
+    ItemPropertyType* value = GetValueForInsertionFromTearOff(item);
+    // Spec: Clears all existing current items from the list and re-initializes
+    // the list to hold the single item specified by the parameter.
+    ListPropertyType* list = ToDerived()->Target();
+    list->Clear();
+    list->Append(value);
     ToDerived()->CommitChange();
-    return CreateItemTearOff(value);
+    return AttachedItemTearOff(value);
   }
 
   ItemTearOffType* getItem(uint32_t index, ExceptionState& exception_state) {
-    ItemPropertyType* value =
-        ToDerived()->Target()->GetItem(index, exception_state);
-    return CreateItemTearOff(value);
+    ListPropertyType* list = ToDerived()->Target();
+    if (index >= list->length()) {
+      SVGPropertyTearOffBase::ThrowIndexSize(exception_state, index,
+                                             list->length());
+      return nullptr;
+    }
+    ItemPropertyType* value = list->at(index);
+    return AttachedItemTearOff(value);
   }
 
   ItemTearOffType* insertItemBefore(ItemTearOffType* item,
@@ -86,10 +97,18 @@ class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
       return nullptr;
     }
     DCHECK(item);
-    ItemPropertyType* value = ToDerived()->Target()->InsertItemBefore(
-        GetValueForInsertionFromTearOff(item), index);
+    ItemPropertyType* value = GetValueForInsertionFromTearOff(item);
+    ListPropertyType* list = ToDerived()->Target();
+    // Spec: If the index is greater than or equal to length, then the new item
+    // is appended to the end of the list.
+    index = std::min(index, list->length());
+    // Spec: Inserts a new item into the list at the specified position. The
+    // index of the item before which the new item is to be inserted. The first
+    // item is number 0. If the index is equal to 0, then the new item is
+    // inserted at the front of the list.
+    list->Insert(index, value);
     ToDerived()->CommitChange();
-    return CreateItemTearOff(value);
+    return AttachedItemTearOff(value);
   }
 
   ItemTearOffType* replaceItem(ItemTearOffType* item,
@@ -99,11 +118,17 @@ class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
       SVGPropertyTearOffBase::ThrowReadOnly(exception_state);
       return nullptr;
     }
+    ListPropertyType* list = ToDerived()->Target();
+    if (index >= list->length()) {
+      SVGPropertyTearOffBase::ThrowIndexSize(exception_state, index,
+                                             list->length());
+      return nullptr;
+    }
     DCHECK(item);
-    ItemPropertyType* value = ToDerived()->Target()->ReplaceItem(
-        GetValueForInsertionFromTearOff(item), index, exception_state);
+    ItemPropertyType* value = GetValueForInsertionFromTearOff(item);
+    list->Replace(index, value);
     ToDerived()->CommitChange();
-    return CreateItemTearOff(value);
+    return AttachedItemTearOff(value);
   }
 
   IndexedPropertySetterResult AnonymousIndexedSetter(
@@ -119,10 +144,16 @@ class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
       SVGPropertyTearOffBase::ThrowReadOnly(exception_state);
       return nullptr;
     }
-    ItemPropertyType* value =
-        ToDerived()->Target()->RemoveItem(index, exception_state);
+    ListPropertyType* list = ToDerived()->Target();
+    if (index >= list->length()) {
+      SVGPropertyTearOffBase::ThrowIndexSize(exception_state, index,
+                                             list->length());
+      return nullptr;
+    }
+    ItemPropertyType* value = list->at(index);
+    list->Remove(index);
     ToDerived()->CommitChange();
-    return CreateItemTearOff(value);
+    return DetachedItemTearOff(value);
   }
 
   ItemTearOffType* appendItem(ItemTearOffType* item,
@@ -132,10 +163,10 @@ class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
       return nullptr;
     }
     DCHECK(item);
-    ItemPropertyType* value = ToDerived()->Target()->AppendItem(
-        GetValueForInsertionFromTearOff(item));
+    ItemPropertyType* value = GetValueForInsertionFromTearOff(item);
+    ToDerived()->Target()->Append(value);
     ToDerived()->CommitChange();
-    return CreateItemTearOff(value);
+    return AttachedItemTearOff(value);
   }
 
  protected:
@@ -170,15 +201,15 @@ class SVGListPropertyTearOffHelper : public SVGPropertyTearOff<ListProperty> {
     item_tear_off->Bind(ToDerived()->GetBinding());
     return item;
   }
-
-  ItemTearOffType* CreateItemTearOff(ItemPropertyType* value) {
-    if (!value)
-      return nullptr;
-
-    if (value->OwnerList() == ToDerived()->Target()) {
-      return MakeGarbageCollected<ItemTearOffType>(
-          value, ToDerived()->GetBinding(), ToDerived()->PropertyIsAnimVal());
-    }
+  ItemTearOffType* AttachedItemTearOff(ItemPropertyType* value) {
+    DCHECK(value);
+    DCHECK_EQ(value->OwnerList(), ToDerived()->Target());
+    return MakeGarbageCollected<ItemTearOffType>(
+        value, ToDerived()->GetBinding(), ToDerived()->PropertyIsAnimVal());
+  }
+  ItemTearOffType* DetachedItemTearOff(ItemPropertyType* value) {
+    DCHECK(value);
+    DCHECK_EQ(value->OwnerList(), nullptr);
     return MakeGarbageCollected<ItemTearOffType>(value, nullptr,
                                                  kPropertyIsNotAnimVal);
   }

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

@@ -61,6 +61,14 @@ void SVGPropertyTearOffBase::ThrowReadOnly(ExceptionState& exception_state) {
       ExceptionMessages::ReadOnly());
 }
 
+void SVGPropertyTearOffBase::ThrowIndexSize(ExceptionState& exception_state,
+                                            uint32_t index,
+                                            uint32_t max_bound) {
+  exception_state.ThrowDOMException(
+      DOMExceptionCode::kIndexSizeError,
+      ExceptionMessages::IndexExceedsMaximumBound("index", index, max_bound));
+}
+
 void SVGPropertyTearOffBase::Bind(SVGAnimatedPropertyBase* binding) {
   DCHECK(!IsImmutable());
   DCHECK(binding);

third_party/blink/renderer/core/svg/properties/svg_property_tear_off.h

@@ -64,6 +64,7 @@ class SVGPropertyTearOffBase : public ScriptWrappable {
   void Trace(Visitor*) const override;
 
   static void ThrowReadOnly(ExceptionState&);
+  static void ThrowIndexSize(ExceptionState&, uint32_t, uint32_t);
 
  protected:
   SVGPropertyTearOffBase(SVGAnimatedPropertyBase* binding,

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

@@ -23,7 +23,6 @@
 #include "third_party/blink/renderer/core/svg/svg_animate_element.h"
 #include "third_party/blink/renderer/core/svg/svg_parser_utilities.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 
 namespace blink {
 
@@ -43,10 +42,6 @@ SVGPropertyBase* SVGLengthList::CloneForAnimation(const String& value) const {
   return ret;
 }
 
-String SVGLengthList::ValueAsString() const {
-  return SVGListPropertyHelper<SVGLengthList, SVGLength>::SerializeList();
-}
-
 template <typename CharType>
 SVGParsingError SVGLengthList::ParseInternal(const CharType*& ptr,
                                              const CharType* end) {

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

@@ -53,7 +53,6 @@ class SVGLengthList final
   // SVGPropertyBase:
   SVGPropertyBase* CloneForAnimation(const String&) const override;
   SVGLengthList* Clone() override;
-  String ValueAsString() const override;
   SVGLengthMode UnitMode() const { return mode_; }
 
   void Add(SVGPropertyBase*, SVGElement*) override;
@@ -67,7 +66,6 @@ class SVGLengthList final
   float CalculateDistance(SVGPropertyBase* to, SVGElement*) override;
 
   static AnimatedPropertyType ClassType() { return kAnimatedLengthList; }
-  AnimatedPropertyType GetType() const override { return ClassType(); }
 
  private:
   // Create SVGLength items used to adjust the list length

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

@@ -23,7 +23,6 @@
 #include "third_party/blink/renderer/core/svg/svg_animate_element.h"
 #include "third_party/blink/renderer/core/svg/svg_parser_utilities.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -32,10 +31,6 @@ SVGNumberList::SVGNumberList() = default;
 
 SVGNumberList::~SVGNumberList() = default;
 
-String SVGNumberList::ValueAsString() const {
-  return SVGListPropertyHelper<SVGNumberList, SVGNumber>::SerializeList();
-}
-
 template <typename CharType>
 SVGParsingError SVGNumberList::Parse(const CharType*& ptr,
                                      const CharType* end) {

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

@@ -51,8 +51,6 @@ class SVGNumberList final
   SVGParsingError SetValueAsString(const String&);
 
   // SVGPropertyBase:
-  String ValueAsString() const override;
-
   void Add(SVGPropertyBase*, SVGElement*) override;
   void CalculateAnimatedValue(const SVGAnimateElement&,
                               float percentage,
@@ -64,7 +62,6 @@ class SVGNumberList final
   float CalculateDistance(SVGPropertyBase* to, SVGElement*) override;
 
   static AnimatedPropertyType ClassType() { return kAnimatedNumberList; }
-  AnimatedPropertyType GetType() const override { return ClassType(); }
 
   Vector<float> ToFloatVector() const;
 

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

@@ -34,6 +34,7 @@
 #include "third_party/blink/renderer/core/svg/properties/svg_property_helper.h"
 #include "third_party/blink/renderer/core/svg/svg_parsing_error.h"
 #include "third_party/blink/renderer/platform/geometry/float_point.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace blink {
 
@@ -82,6 +83,13 @@ class SVGPoint final : public SVGPropertyHelper<SVGPoint> {
   FloatPoint value_;
 };
 
+template <>
+struct DowncastTraits<SVGPoint> {
+  static bool AllowFrom(const SVGPropertyBase& value) {
+    return value.GetType() == SVGPoint::ClassType();
+  }
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_POINT_H_

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

@@ -24,7 +24,6 @@
 #include "third_party/blink/renderer/core/svg/svg_parser_utilities.h"
 #include "third_party/blink/renderer/platform/geometry/float_point.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -33,10 +32,6 @@ SVGPointList::SVGPointList() = default;
 
 SVGPointList::~SVGPointList() = default;
 
-String SVGPointList::ValueAsString() const {
-  return SVGListPropertyHelper<SVGPointList, SVGPoint>::SerializeList();
-}
-
 template <typename CharType>
 SVGParsingError SVGPointList::Parse(const CharType*& ptr, const CharType* end) {
   if (!SkipOptionalSVGSpaces(ptr, end))

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

@@ -51,8 +51,6 @@ class SVGPointList final
   SVGParsingError SetValueAsString(const String&);
 
   // SVGPropertyBase:
-  String ValueAsString() const override;
-
   void Add(SVGPropertyBase*, SVGElement*) override;
   void CalculateAnimatedValue(const SVGAnimateElement&,
                               float percentage,
@@ -64,7 +62,6 @@ class SVGPointList final
   float CalculateDistance(SVGPropertyBase* to, SVGElement*) override;
 
   static AnimatedPropertyType ClassType() { return kAnimatedPoints; }
-  AnimatedPropertyType GetType() const override { return ClassType(); }
 
  private:
   template <typename CharType>

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

@@ -50,8 +50,8 @@ Path SVGPolyElement::AsPathFromPoints() const {
   if (points_value->IsEmpty())
     return path;
 
-  const auto* it = points_value->begin();
-  const auto* it_end = points_value->end();
+  auto it = points_value->begin();
+  auto it_end = points_value->end();
   DCHECK(it != it_end);
   path.MoveTo((*it)->Value());
   ++it;

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

@@ -24,6 +24,7 @@
 #include "third_party/blink/renderer/core/svg/properties/svg_property.h"
 #include "third_party/blink/renderer/platform/geometry/float_point.h"
 #include "third_party/blink/renderer/platform/transforms/affine_transform.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -108,6 +109,13 @@ class SVGTransform final : public SVGPropertyBase {
   AffineTransform matrix_;
 };
 
+template <>
+struct DowncastTraits<SVGTransform> {
+  static bool AllowFrom(const SVGPropertyBase& value) {
+    return value.GetType() == SVGTransform::ClassType();
+  }
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_TRANSFORM_H_

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

@@ -32,10 +32,8 @@
 #include "third_party/blink/renderer/core/svg/svg_animate_element.h"
 #include "third_party/blink/renderer/core/svg/svg_parser_utilities.h"
 #include "third_party/blink/renderer/core/svg/svg_transform_distance.h"
-#include "third_party/blink/renderer/core/svg_names.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/wtf/text/parsing_utilities.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -183,7 +181,7 @@ SVGTransformList::~SVGTransformList() = default;
 
 AffineTransform SVGTransformList::Concatenate() const {
   AffineTransform result;
-  for (const auto& item : *this)
+  for (const auto* item : *this)
     result *= item->Matrix();
   return result;
 }
@@ -281,7 +279,7 @@ const CSSValue* SVGTransformList::CssValue() const {
     list->Append(*CreateTransformCSSValue(*at(0)));
     return list;
   }
-  for (const auto& item : *this)
+  for (const auto* item : *this)
     list->Append(*CreateTransformCSSValue(*item));
   return list;
 }
@@ -382,10 +380,6 @@ SVGTransformType ParseTransformType(const String& string) {
   return ParseAndSkipTransformType(ptr, end);
 }
 
-String SVGTransformList::ValueAsString() const {
-  return SVGListPropertyHelper<SVGTransformList, SVGTransform>::SerializeList();
-}
-
 SVGParsingError SVGTransformList::SetValueAsString(const String& value) {
   if (value.IsEmpty()) {
     Clear();
@@ -429,8 +423,8 @@ void SVGTransformList::Add(SVGPropertyBase* other,
   SVGTransform* to_transform = other_list->at(0);
 
   DCHECK_EQ(from_transform->TransformType(), to_transform->TransformType());
-  Initialize(
-      SVGTransformDistance::AddSVGTransforms(from_transform, to_transform));
+  Clear();
+  Append(SVGTransformDistance::AddSVGTransforms(from_transform, to_transform));
 }
 
 void SVGTransformList::CalculateAnimatedValue(
@@ -474,7 +468,8 @@ void SVGTransformList::CalculateAnimatedValue(
           .ScaledDistance(percentage)
           .AddToSVGTransform(effective_from);
   if (animation_element.GetAnimationMode() == kToAnimation) {
-    Initialize(current_transform);
+    Clear();
+    Append(current_transform);
     return;
   }
   // Never resize the animatedTransformList to the toList size, instead either

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

@@ -54,7 +54,6 @@ class SVGTransformList final
 
   // SVGPropertyBase:
   SVGPropertyBase* CloneForAnimation(const String&) const override;
-  String ValueAsString() const override;
   SVGParsingError SetValueAsString(const String&);
   bool Parse(const UChar*& ptr, const UChar* end);
   bool Parse(const LChar*& ptr, const LChar* end);
@@ -70,7 +69,6 @@ class SVGTransformList final
   float CalculateDistance(SVGPropertyBase* to, SVGElement*) override;
 
   static AnimatedPropertyType ClassType() { return kAnimatedTransformList; }
-  AnimatedPropertyType GetType() const override { return ClassType(); }
 
   const CSSValue* CssValue() const;
 

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

@@ -62,8 +62,9 @@ SVGTransformTearOff* SVGTransformListTearOff::consolidate(
     return nullptr;
   auto* concatenated_transform =
       MakeGarbageCollected<SVGTransform>(transform_list->Concatenate());
-  transform_list->Initialize(concatenated_transform);
-  return CreateItemTearOff(concatenated_transform);
+  transform_list->Clear();
+  transform_list->Append(concatenated_transform);
+  return AttachedItemTearOff(concatenated_transform);
 }
 
 }  // namespace blink