Skip to content

Commit

Permalink
Bug 1731374 - Add string storage to AccAttributes with move semantics…
Browse files Browse the repository at this point in the history
…. r=Jamie

Differential Revision: https://phabricator.services.mozilla.com/D126012
  • Loading branch information
eeejay committed Sep 28, 2021
1 parent d4a231a commit 1de5b0a
Show file tree
Hide file tree
Showing 14 changed files with 118 additions and 62 deletions.
3 changes: 3 additions & 0 deletions accessible/base/AccAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ void AccAttributes::StringFromValueAndName(nsAtom* aAttrName,
},
[&aValueString](const DeleteEntry& val) {
aValueString.Append(u"-delete-entry-");
},
[&aValueString](const UniquePtr<nsString>& val) {
aValueString.Assign(*val);
});
}

Expand Down
42 changes: 31 additions & 11 deletions accessible/base/AccAttributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ struct DeleteEntry {
};

class AccAttributes {
friend struct IPC::ParamTraits<AccAttributes*>;
using AttrValueType =
Variant<bool, float, double, int32_t, RefPtr<nsAtom>, nsTArray<int32_t>,
CSSCoord, FontSize, Color, DeleteEntry>;
CSSCoord, FontSize, Color, DeleteEntry, UniquePtr<nsString>>;
static_assert(sizeof(AttrValueType) <= 16);
using AtomVariantMap = nsTHashMap<nsRefPtrHashKey<nsAtom>, AttrValueType>;

Expand All @@ -70,9 +69,14 @@ class AccAttributes {
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, nsString&& aAttrValue) {
UniquePtr<nsString> value = MakeUnique<nsString>();
*value = std::forward<nsString>(aAttrValue);
mData.InsertOrUpdate(aAttrName, AsVariant(std::move(value)));
}

void SetAttributeStringCopy(nsAtom* aAttrName, nsString aAttrValue) {
SetAttribute(aAttrName, std::move(aAttrValue));
}

void SetAttribute(nsAtom* aAttrName, nsAtom* aAttrValue) {
Expand All @@ -82,9 +86,16 @@ class AccAttributes {
template <typename T>
Maybe<const T&> GetAttribute(nsAtom* aAttrName) {
if (auto value = mData.Lookup(aAttrName)) {
if (value->is<T>()) {
const T& val = value->as<T>();
return SomeRef(val);
if constexpr (std::is_same_v<nsString, T>) {
if (value->is<UniquePtr<nsString>>()) {
const T& val = *(value->as<UniquePtr<nsString>>().get());
return SomeRef(val);
}
} else {
if (value->is<T>()) {
const T& val = value->as<T>();
return SomeRef(val);
}
}
}
return Nothing();
Expand All @@ -111,9 +122,16 @@ class AccAttributes {

template <typename T>
Maybe<const T&> Value() {
if (mValue->is<T>()) {
const T& val = mValue->as<T>();
return SomeRef(val);
if constexpr (std::is_same_v<nsString, T>) {
if (mValue->is<UniquePtr<nsString>>()) {
const T& val = *(mValue->as<UniquePtr<nsString>>().get());
return SomeRef(val);
}
} else {
if (mValue->is<T>()) {
const T& val = mValue->as<T>();
return SomeRef(val);
}
}
return Nothing();
}
Expand Down Expand Up @@ -176,6 +194,8 @@ class AccAttributes {
nsAString& aValueString);

AtomVariantMap mData;

friend struct IPC::ParamTraits<AccAttributes*>;
};

} // namespace a11y
Expand Down
9 changes: 4 additions & 5 deletions accessible/base/TextAttrs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ bool TextAttrsMgr::LangTextAttr::GetValueFor(LocalAccessible* aAccessible,

void TextAttrsMgr::LangTextAttr::ExposeValue(AccAttributes* aAttributes,
const nsString& aValue) {
aAttributes->SetAttribute(nsGkAtoms::language, aValue);
aAttributes->SetAttributeStringCopy(nsGkAtoms::language, aValue);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -397,7 +397,7 @@ bool TextAttrsMgr::FontFamilyTextAttr::GetValueFor(LocalAccessible* aAccessible,

void TextAttrsMgr::FontFamilyTextAttr::ExposeValue(AccAttributes* aAttributes,
const nsString& aValue) {
aAttributes->SetAttribute(nsGkAtoms::font_family, aValue);
aAttributes->SetAttributeStringCopy(nsGkAtoms::font_family, aValue);
}

bool TextAttrsMgr::FontFamilyTextAttr::GetFontFamily(nsIFrame* aFrame,
Expand Down Expand Up @@ -498,14 +498,13 @@ void TextAttrsMgr::FontStyleTextAttr::ExposeValue(
aAttributes->SetAttribute(nsGkAtoms::font_style, atom);
} else {
auto angle = aValue.ObliqueAngle();
nsAutoString string;
string.AppendLiteral("oblique");
nsString string(u"oblique"_ns);
if (angle != FontSlantStyle::kDefaultAngle) {
string.AppendLiteral(" ");
nsStyleUtil::AppendCSSNumber(angle, string);
string.AppendLiteral("deg");
}
aAttributes->SetAttribute(nsGkAtoms::font_style, string);
aAttributes->SetAttribute(nsGkAtoms::font_style, std::move(string));
}
}

Expand Down
9 changes: 5 additions & 4 deletions accessible/base/nsAccUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ void nsAccUtils::SetLiveContainerAttributes(AccAttributes* aAttributes,
HasDefinedARIAToken(ancestor, nsGkAtoms::aria_relevant) &&
ancestor->AsElement()->GetAttr(kNameSpaceID_None,
nsGkAtoms::aria_relevant, relevant)) {
aAttributes->SetAttribute(nsGkAtoms::containerRelevant, relevant);
aAttributes->SetAttributeStringCopy(nsGkAtoms::containerRelevant,
relevant);
}

// container-live, and container-live-role attributes
Expand All @@ -124,10 +125,10 @@ void nsAccUtils::SetLiveContainerAttributes(AccAttributes* aAttributes,
}

if (!live.IsEmpty()) {
aAttributes->SetAttribute(nsGkAtoms::containerLive, live);
aAttributes->SetAttributeStringCopy(nsGkAtoms::containerLive, live);
if (role) {
aAttributes->SetAttribute(nsGkAtoms::containerLiveRole,
role->ARIARoleString());
role->roleAtom);
}
}
}
Expand All @@ -143,7 +144,7 @@ void nsAccUtils::SetLiveContainerAttributes(AccAttributes* aAttributes,
if (busy.IsEmpty() && HasDefinedARIAToken(ancestor, nsGkAtoms::aria_busy) &&
ancestor->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::aria_busy,
busy)) {
aAttributes->SetAttribute(nsGkAtoms::containerBusy, busy);
aAttributes->SetAttributeStringCopy(nsGkAtoms::containerBusy, busy);
}

if (ancestor == topEl) {
Expand Down
5 changes: 2 additions & 3 deletions accessible/base/nsAccessibilityService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1469,15 +1469,14 @@ void nsAccessibilityService::MarkupAttributes(
continue;
}

nsAutoString value;

nsString value;
if (aContent->IsElement()) {
aContent->AsElement()->GetAttr(kNameSpaceID_None, info->DOMAttrName,
value);
}

if (!value.IsEmpty()) {
aAttributes->SetAttribute(info->name, value);
aAttributes->SetAttribute(info->name, std::move(value));
}

continue;
Expand Down
2 changes: 1 addition & 1 deletion accessible/generic/ARIAGridAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ already_AddRefed<AccAttributes> ARIAGridCellAccessible::NativeAttributes() {

#ifdef DEBUG
RefPtr<nsAtom> cppClass = NS_Atomize(u"cppclass"_ns);
attributes->SetAttribute(cppClass, u"ARIAGridCellAccessible"_ns);
attributes->SetAttributeStringCopy(cppClass, u"ARIAGridCellAccessible"_ns);
#endif

return attributes.forget();
Expand Down
4 changes: 2 additions & 2 deletions accessible/generic/DocAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ already_AddRefed<AccAttributes> DocAccessible::Attributes() {
// Override ARIA object attributes from outerdoc.
aria::AttrIterator attribIter(mParent->GetContent());
while (attribIter.Next()) {
nsAutoString value;
nsString value;
attribIter.AttrValue(value);
attributes->SetAttribute(attribIter.AttrName(), value);
attributes->SetAttribute(attribIter.AttrName(), std::move(value));
}

return attributes.forget();
Expand Down
4 changes: 2 additions & 2 deletions accessible/generic/ImageAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ nsIntSize ImageAccessible::Size() { return Bounds().Size(); }
already_AddRefed<AccAttributes> ImageAccessible::NativeAttributes() {
RefPtr<AccAttributes> attributes = LinkableAccessible::NativeAttributes();

nsAutoString src;
nsString src;
mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::src, src);
if (!src.IsEmpty()) attributes->SetAttribute(nsGkAtoms::src, src);
if (!src.IsEmpty()) attributes->SetAttribute(nsGkAtoms::src, std::move(src));

return attributes.forget();
}
Expand Down
39 changes: 20 additions & 19 deletions accessible/generic/LocalAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -986,10 +986,10 @@ already_AddRefed<AccAttributes> LocalAccessible::Attributes() {
if (!HasOwnContent() || !mContent->IsElement()) return attributes.forget();

// 'xml-roles' attribute coming from ARIA.
nsAutoString xmlRoles;
nsString xmlRoles;
if (mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::role,
xmlRoles)) {
attributes->SetAttribute(nsGkAtoms::xmlroles, xmlRoles);
attributes->SetAttribute(nsGkAtoms::xmlroles, std::move(xmlRoles));
} else if (nsAtom* landmark = LandmarkRole()) {
// 'xml-roles' attribute for landmark.
attributes->SetAttribute(nsGkAtoms::xmlroles, landmark);
Expand All @@ -998,23 +998,23 @@ already_AddRefed<AccAttributes> LocalAccessible::Attributes() {
// Expose object attributes from ARIA attributes.
aria::AttrIterator attribIter(mContent);
while (attribIter.Next()) {
nsAutoString value;
nsString value;
attribIter.AttrValue(value);
attributes->SetAttribute(attribIter.AttrName(), value);
attributes->SetAttribute(attribIter.AttrName(), std::move(value));
}

// If there is no aria-live attribute then expose default value of 'live'
// object attribute used for ARIA role of this accessible.
const nsRoleMapEntry* roleMapEntry = ARIARoleMap();
if (roleMapEntry) {
if (roleMapEntry->Is(nsGkAtoms::searchbox)) {
attributes->SetAttribute(nsGkAtoms::textInputType, u"search"_ns);
attributes->SetAttribute(nsGkAtoms::textInputType, nsGkAtoms::search);
}

if (!attributes->HasAttribute(nsGkAtoms::aria_live)) {
nsAutoString live;
nsString live;
if (nsAccUtils::GetLiveAttrValue(roleMapEntry->liveAttRule, live)) {
attributes->SetAttribute(nsGkAtoms::aria_live, live);
attributes->SetAttribute(nsGkAtoms::aria_live, std::move(live));
}
}
}
Expand All @@ -1030,9 +1030,9 @@ already_AddRefed<AccAttributes> LocalAccessible::NativeAttributes() {
// to expose traditional Value() information such as URL's on links and
// documents, or text in an input.
if (HasNumericValue()) {
nsAutoString valuetext;
nsString valuetext;
Value(valuetext);
attributes->SetAttribute(nsGkAtoms::aria_valuetext, valuetext);
attributes->SetAttribute(nsGkAtoms::aria_valuetext, std::move(valuetext));
}

// Expose checkable object attribute if the accessible has checkable state
Expand Down Expand Up @@ -1076,16 +1076,16 @@ already_AddRefed<AccAttributes> LocalAccessible::NativeAttributes() {

if (!mContent->IsElement()) return attributes.forget();

nsAutoString id;
nsString id;
if (nsCoreUtils::GetID(mContent, id)) {
attributes->SetAttribute(nsGkAtoms::id, id);
attributes->SetAttribute(nsGkAtoms::id, std::move(id));
}

// Expose class because it may have useful microformat information.
nsAutoString _class;
nsString _class;
if (mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::_class,
_class)) {
attributes->SetAttribute(nsGkAtoms::_class, _class);
attributes->SetAttribute(nsGkAtoms::_class, std::move(_class));
}

// Expose tag.
Expand Down Expand Up @@ -1137,10 +1137,11 @@ already_AddRefed<AccAttributes> LocalAccessible::NativeAttributes() {

// Expose data-at-shortcutkeys attribute for web applications and virtual
// cursors. Currently mostly used by JAWS.
nsAutoString atShortcutKeys;
nsString atShortcutKeys;
if (mContent->AsElement()->GetAttr(
kNameSpaceID_None, nsGkAtoms::dataAtShortcutkeys, atShortcutKeys)) {
attributes->SetAttribute(nsGkAtoms::dataAtShortcutkeys, atShortcutKeys);
attributes->SetAttribute(nsGkAtoms::dataAtShortcutkeys,
std::move(atShortcutKeys));
}

return attributes.forget();
Expand Down Expand Up @@ -3022,7 +3023,7 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache(
RefPtr<AccAttributes> fields = new AccAttributes();

if (aCacheDomain & CacheDomain::NameAndDescription) {
nsAutoString name;
nsString name;
int32_t nameFlag = Name(name);
if (nameFlag != eNameOK) {
fields->SetAttribute(nsGkAtoms::explicit_name, nameFlag);
Expand All @@ -3031,15 +3032,15 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache(
}

if (!name.IsEmpty()) {
fields->SetAttribute(nsGkAtoms::name, name);
fields->SetAttribute(nsGkAtoms::name, std::move(name));
} else if (aUpdateType == CacheUpdateType::Update) {
fields->SetAttribute(nsGkAtoms::name, DeleteEntry());
}

nsAutoString description;
nsString description;
Description(description);
if (!description.IsEmpty()) {
fields->SetAttribute(nsGkAtoms::description, description);
fields->SetAttribute(nsGkAtoms::description, std::move(description));
} else if (aUpdateType == CacheUpdateType::Update) {
fields->SetAttribute(nsGkAtoms::description, DeleteEntry());
}
Expand Down
11 changes: 6 additions & 5 deletions accessible/html/HTMLFormControlAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ already_AddRefed<AccAttributes> HTMLTextFieldAccessible::NativeAttributes() {

// Expose type for text input elements as it gives some useful context,
// especially for mobile.
nsAutoString type;
nsString type;
// In the case of this element being part of a binding, the binding's
// parent's type should have precedence. For example an input[type=number]
// has an embedded anonymous input[type=text] (along with spinner buttons).
Expand All @@ -272,21 +272,22 @@ already_AddRefed<AccAttributes> HTMLTextFieldAccessible::NativeAttributes() {
nsGkAtoms::type, type)) ||
mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::type,
type)) {
attributes->SetAttribute(nsGkAtoms::textInputType, type);
if (!ARIARoleMap() && type.EqualsLiteral("search")) {
attributes->SetAttribute(nsGkAtoms::xmlroles, u"searchbox"_ns);
attributes->SetAttribute(nsGkAtoms::xmlroles, nsGkAtoms::searchbox);
}
attributes->SetAttribute(nsGkAtoms::textInputType, std::move(type));
}

// If this element has the placeholder attribute set,
// and if that is not identical to the name, expose it as an object attribute.
nsAutoString placeholderText;
nsString placeholderText;
if (mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::placeholder,
placeholderText)) {
nsAutoString name;
const_cast<HTMLTextFieldAccessible*>(this)->Name(name);
if (!name.Equals(placeholderText)) {
attributes->SetAttribute(nsGkAtoms::placeholder, placeholderText);
attributes->SetAttribute(nsGkAtoms::placeholder,
std::move(placeholderText));
}
}

Expand Down
4 changes: 2 additions & 2 deletions accessible/html/HTMLFormControlAccessible.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,10 @@ class HTMLDateTimeAccessible : public AccessibleWrap {
RefPtr<AccAttributes> attributes = AccessibleWrap::NativeAttributes();
// Unfortunately, an nsStaticAtom can't be passed as a
// template argument, so fetch the type from the DOM.
nsAutoString type;
nsString type;
if (mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::type,
type)) {
attributes->SetAttribute(nsGkAtoms::textInputType, type);
attributes->SetAttribute(nsGkAtoms::textInputType, std::move(type));
}
return attributes.forget();
}
Expand Down
Loading

0 comments on commit 1de5b0a

Please sign in to comment.