Bug 1731154 - Add move semantics to AccAttributes and use value references to avoid copies. r=morgan

Changed the array type to nsTArray to avoid copies and get compile-time
errors if we ever try to do that. To set an array as a value, it must be
moved.

Differential Revision: https://phabricator.services.mozilla.com/D125899

accessible/base/AccAttributes.cpp

@@ -35,7 +35,7 @@ void AccAttributes::StringFromValueAndName(nsAtom* aAttrName,
       [&aValueString](const RefPtr<nsAtom>& val) {
         val->ToString(aValueString);
       },
-      [&aValueString](const CopyableTArray<int32_t>& val) {
+      [&aValueString](const nsTArray<int32_t>& val) {
         for (size_t i = 0; i < val.Length() - 1; i++) {
           aValueString.AppendInt(val[i]);
           aValueString.Append(u", ");
@@ -59,12 +59,12 @@ void AccAttributes::StringFromValueAndName(nsAtom* aAttrName,
 }
 
 void AccAttributes::Update(AccAttributes* aOther) {
-  for (auto entry : *aOther) {
-    if (entry.mValue->is<DeleteEntry>()) {
-      mData.Remove(entry.mName);
-      continue;
+  for (auto iter = aOther->mData.Iter(); !iter.Done(); iter.Next()) {
+    if (iter.Data().is<DeleteEntry>()) {
+      mData.Remove(iter.Key());
+    } else {
+      mData.InsertOrUpdate(iter.Key(), std::move(iter.Data()));
     }
-
-    mData.InsertOrUpdate(entry.mName, *entry.mValue);
+    iter.Remove();
   }
 }

accessible/base/AccAttributes.h

@@ -45,8 +45,8 @@ struct DeleteEntry {
 class AccAttributes {
   friend struct IPC::ParamTraits<AccAttributes*>;
   using AttrValueType =
-      Variant<bool, float, double, int32_t, RefPtr<nsAtom>,
-              CopyableTArray<int32_t>, CSSCoord, FontSize, Color, DeleteEntry>;
+      Variant<bool, float, double, int32_t, RefPtr<nsAtom>, nsTArray<int32_t>,
+              CSSCoord, FontSize, Color, DeleteEntry>;
   static_assert(sizeof(AttrValueType) <= 16);
   using AtomVariantMap = nsTHashMap<nsRefPtrHashKey<nsAtom>, AttrValueType>;
 
@@ -62,30 +62,29 @@ class AccAttributes {
 
   NS_INLINE_DECL_REFCOUNTING(mozilla::a11y::AccAttributes)
 
-  template <typename T>
-  void SetAttribute(nsAtom* aAttrName, const T& aAttrValue) {
-    if constexpr (std::is_base_of_v<nsAtom, std::remove_pointer_t<T>>) {
-      mData.InsertOrUpdate(aAttrName, AsVariant(RefPtr<nsAtom>(aAttrValue)));
-    } else if constexpr (std::is_base_of_v<nsAString, T> ||
-                         std::is_base_of_v<nsLiteralString, T>) {
-      RefPtr<nsAtom> atomValue = NS_Atomize(aAttrValue);
-      mData.InsertOrUpdate(aAttrName, AsVariant(atomValue));
-    } else {
-      mData.InsertOrUpdate(aAttrName, AsVariant(aAttrValue));
-    }
+  template <typename T,
+            typename std::enable_if<!std::is_convertible_v<T, nsString> &&
+                                        !std::is_convertible_v<T, nsAtom*>,
+                                    bool>::type = true>
+  void SetAttribute(nsAtom* aAttrName, T&& aAttrValue) {
+    mData.InsertOrUpdate(aAttrName, AsVariant(std::forward<T>(aAttrValue)));
+  }
+
+  void SetAttribute(nsAtom* aAttrName, const nsAString& aAttrValue) {
+    RefPtr<nsAtom> atomValue = NS_Atomize(aAttrValue);
+    mData.InsertOrUpdate(aAttrName, AsVariant(atomValue));
+  }
+
+  void SetAttribute(nsAtom* aAttrName, nsAtom* aAttrValue) {
+    mData.InsertOrUpdate(aAttrName, AsVariant(RefPtr<nsAtom>(aAttrValue)));
   }
 
   template <typename T>
-  Maybe<T> GetAttribute(nsAtom* aAttrName) {
+  Maybe<const T&> GetAttribute(nsAtom* aAttrName) {
     if (auto value = mData.Lookup(aAttrName)) {
-      if constexpr (std::is_base_of_v<nsAtom, std::remove_pointer_t<T>>) {
-        if (value->is<RefPtr<nsAtom>>()) {
-          return Some(value->as<RefPtr<nsAtom>>().get());
-        }
-      } else {
-        if (value->is<T>()) {
-          return Some(value->as<T>());
-        }
+      if (value->is<T>()) {
+        const T& val = value->as<T>();
+        return SomeRef(val);
       }
     }
     return Nothing();
@@ -98,6 +97,8 @@ class AccAttributes {
 
   uint32_t Count() const { return mData.Count(); }
 
+  // Update one instance with the entries in another. The supplied AccAttributes
+  // will be emptied.
   void Update(AccAttributes* aOther);
 
   // An entry class for our iterator.
@@ -109,15 +110,10 @@ class AccAttributes {
     nsAtom* Name() { return mName; }
 
     template <typename T>
-    Maybe<T> Value() {
-      if constexpr (std::is_base_of_v<nsAtom, std::remove_pointer_t<T>>) {
-        if (mValue->is<RefPtr<nsAtom>>()) {
-          return Some(mValue->as<RefPtr<nsAtom>>().get());
-        }
-      } else {
-        if (mValue->is<T>()) {
-          return Some(mValue->as<T>());
-        }
+    Maybe<const T&> Value() {
+      if (mValue->is<T>()) {
+        const T& val = mValue->as<T>();
+        return SomeRef(val);
       }
       return Nothing();
     }

accessible/ipc/IPCTypes.h

@@ -133,7 +133,7 @@ struct ParamTraits<mozilla::a11y::AccAttributes*> {
       if (!ReadParam(aMsg, aIter, &val)) {
         return false;
       }
-      (*aResult)->mData.InsertOrUpdate(key, val);
+      (*aResult)->mData.InsertOrUpdate(key, std::move(val));
     }
     return true;
   }

accessible/mac/GeckoTextMarker.mm

@@ -450,7 +450,7 @@ static NSDictionary* StringAttributesFromAttributes(AccAttributes* aAttributes,
       }
     } else if (iter.Name() == nsGkAtoms::invalid) {
       // XXX: There is currently no attribute for grammar
-      if (Maybe<nsAtom*> value = iter.Value<nsAtom*>()) {
+      if (auto value = iter.Value<RefPtr<nsAtom>>()) {
         if (*value == nsGkAtoms::spelling) {
           [attrDict setObject:@YES
                        forKey:NSAccessibilityMarkedMisspelledTextAttribute];