Commit b90d7299 authored by Aaron Leventhal's avatar Aaron Leventhal Committed by Commit Bot

Check AX cache if labelled before fetching labels

HTML labelable elements can cause extreme slowness while loading in some
cases. We should only call html_element->labels() if the current element
is known to have labels (we can check our relationship cache).

Bug: 976849
Change-Id: Ia5e0e312b3a5e1d2d2d4cfdc5249f6a58b38147c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1669911
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672099}
parent 6c21f7a3
...@@ -2876,7 +2876,9 @@ String AXNodeObject::NativeTextAlternative( ...@@ -2876,7 +2876,9 @@ String AXNodeObject::NativeTextAlternative(
name_sources->back().native_source = kAXTextFromNativeHTMLLabel; name_sources->back().native_source = kAXTextFromNativeHTMLLabel;
} }
LabelsNodeList* labels = html_element->labels(); LabelsNodeList* labels = nullptr;
if (AXObjectCache().MayHaveHTMLLabel(*html_element))
labels = html_element->labels();
if (labels && labels->length() > 0) { if (labels && labels->length() > 0) {
HeapVector<Member<Element>> label_elements; HeapVector<Member<Element>> label_elements;
for (unsigned label_index = 0; label_index < labels->length(); for (unsigned label_index = 0; label_index < labels->length();
......
...@@ -1017,6 +1017,19 @@ void AXObjectCacheImpl::UpdateAriaOwns( ...@@ -1017,6 +1017,19 @@ void AXObjectCacheImpl::UpdateAriaOwns(
relation_cache_->UpdateAriaOwns(owner, id_vector, owned_children); relation_cache_->UpdateAriaOwns(owner, id_vector, owned_children);
} }
bool AXObjectCacheImpl::MayHaveHTMLLabel(const HTMLElement& elem) {
// Return false if this type of element will not accept a <label for> label.
if (!elem.IsLabelable())
return false;
// Return true if a <label for> pointed to this element at some point.
if (relation_cache_->MayHaveHTMLLabelViaForAttribute(elem))
return true;
// Return true if any amcestor is a label, as in <label><input></label>.
return Traversal<HTMLLabelElement>::FirstAncestor(elem);
}
void AXObjectCacheImpl::CheckedStateChanged(Node* node) { void AXObjectCacheImpl::CheckedStateChanged(Node* node) {
PostNotification(node, ax::mojom::Event::kCheckedStateChanged); PostNotification(node, ax::mojom::Event::kCheckedStateChanged);
} }
...@@ -1388,7 +1401,8 @@ void AXObjectCacheImpl::HandleValidationMessageVisibilityChanged( ...@@ -1388,7 +1401,8 @@ void AXObjectCacheImpl::HandleValidationMessageVisibilityChanged(
} }
void AXObjectCacheImpl::LabelChangedWithCleanLayout(Element* element) { void AXObjectCacheImpl::LabelChangedWithCleanLayout(Element* element) {
TextChangedWithCleanLayout(ToHTMLLabelElement(element)->control()); // Will call back to TextChanged() when done updating relation cache.
relation_cache_->LabelChanged(element);
} }
void AXObjectCacheImpl::InlineTextBoxesUpdated( void AXObjectCacheImpl::InlineTextBoxesUpdated(
......
...@@ -233,6 +233,8 @@ class MODULES_EXPORT AXObjectCacheImpl ...@@ -233,6 +233,8 @@ class MODULES_EXPORT AXObjectCacheImpl
const Vector<String>& id_vector, const Vector<String>& id_vector,
HeapVector<Member<AXObject>>& owned_children); HeapVector<Member<AXObject>>& owned_children);
bool MayHaveHTMLLabel(const HTMLElement& elem);
// Synchronously returns whether or not we currently have permission to // Synchronously returns whether or not we currently have permission to
// call AOM event listeners. // call AOM event listeners.
bool CanCallAOMEventListeners() const; bool CanCallAOMEventListeners() const;
......
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
namespace blink { namespace blink {
using namespace html_names;
AXRelationCache::AXRelationCache(AXObjectCacheImpl* object_cache) AXRelationCache::AXRelationCache(AXObjectCacheImpl* object_cache)
: object_cache_(object_cache) {} : object_cache_(object_cache) {}
...@@ -167,6 +169,14 @@ void AXRelationCache::UpdateAriaOwns( ...@@ -167,6 +169,14 @@ void AXRelationCache::UpdateAriaOwns(
validated_owned_child_axids); validated_owned_child_axids);
} }
bool AXRelationCache::MayHaveHTMLLabelViaForAttribute(
const HTMLElement& labelable) {
const AtomicString& id = labelable.GetIdAttribute();
if (id.IsEmpty())
return false;
return all_previously_seen_label_target_ids_.Contains(id);
}
// Fill source_objects with AXObjects for relations pointing to target. // Fill source_objects with AXObjects for relations pointing to target.
void AXRelationCache::GetReverseRelated( void AXRelationCache::GetReverseRelated(
Node* target, Node* target,
...@@ -260,8 +270,12 @@ void AXRelationCache::TextChanged(AXObject* object) { ...@@ -260,8 +270,12 @@ void AXRelationCache::TextChanged(AXObject* object) {
} }
void AXRelationCache::LabelChanged(Node* node) { void AXRelationCache::LabelChanged(Node* node) {
if (HTMLElement* control = ToHTMLLabelElement(node)->control()) const AtomicString& id = ToHTMLElement(node)->FastGetAttribute(kForAttr);
TextChanged(Get(control)); if (!id.IsEmpty()) {
all_previously_seen_label_target_ids_.insert(id);
if (HTMLElement* control = ToHTMLLabelElement(node)->control())
TextChanged(Get(control));
}
} }
} // namespace blink } // namespace blink
...@@ -38,6 +38,9 @@ class AXRelationCache { ...@@ -38,6 +38,9 @@ class AXRelationCache {
const Vector<String>& id_vector, const Vector<String>& id_vector,
HeapVector<Member<AXObject>>& owned_children); HeapVector<Member<AXObject>>& owned_children);
// Return true if any label ever pointed to the element via the for attribute.
bool MayHaveHTMLLabelViaForAttribute(const HTMLElement&);
// Given an element in the DOM tree that was either just added or whose id // Given an element in the DOM tree that was either just added or whose id
// just changed, check to see if another object wants to be its parent due to // just changed, check to see if another object wants to be its parent due to
// aria-owns. If so, update the tree by calling childrenChanged() on the // aria-owns. If so, update the tree by calling childrenChanged() on the
...@@ -54,6 +57,8 @@ class AXRelationCache { ...@@ -54,6 +57,8 @@ class AXRelationCache {
void UpdateReverseRelations(const AXObject* relation_source, void UpdateReverseRelations(const AXObject* relation_source,
const Vector<String>& target_ids); const Vector<String>& target_ids);
void LabelChanged(Node*);
private: private:
// If any object is related to this object via <label for>, aria-owns, // If any object is related to this object via <label for>, aria-owns,
// aria-describedby or aria-labeledby, update the text for the related object. // aria-describedby or aria-labeledby, update the text for the related object.
...@@ -90,13 +95,19 @@ class AXRelationCache { ...@@ -90,13 +95,19 @@ class AXRelationCache {
// and fire the appropriate change events. // and fire the appropriate change events.
HashMap<String, HashSet<AXID>> id_attr_to_related_mapping_; HashMap<String, HashSet<AXID>> id_attr_to_related_mapping_;
// HTML id attributes that at one time havehad a <label for> pointing to it.
// IDs are not necessarily removed from this set. It is not necessary to
// remove IDs as false positives are ok. Being able to determine that a
// labelable element has never had an associated label allows the accessible
// name calculation to be optimized.
HashSet<AtomicString> all_previously_seen_label_target_ids_;
// Helpers that call back into object cache // Helpers that call back into object cache
AXObject* ObjectFromAXID(AXID) const; AXObject* ObjectFromAXID(AXID) const;
AXObject* GetOrCreate(Node*); AXObject* GetOrCreate(Node*);
AXObject* Get(Node*); AXObject* Get(Node*);
void ChildrenChanged(AXObject*); void ChildrenChanged(AXObject*);
void TextChanged(AXObject*); void TextChanged(AXObject*);
void LabelChanged(Node*);
DISALLOW_COPY_AND_ASSIGN(AXRelationCache); DISALLOW_COPY_AND_ASSIGN(AXRelationCache);
}; };
......
...@@ -79,7 +79,9 @@ test(function(t) { ...@@ -79,7 +79,9 @@ test(function(t) {
<div class="container"> <div class="container">
<label>label-wrapping-text7 <label>label-wrapping-text7
<input id="text7" type="text" title="text7-title"> <span> <!-- Also testing ancestor label that is not direct parent -->
<input id="text7" type="text" title="text7-title">
</span>
</label> </label>
</div> </div>
......
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