Commit 4287896c authored by Aaron Leventhal's avatar Aaron Leventhal Committed by Commit Bot

Don't change processed ancestors in DidInsertChildrenOfNode()

A previous CL tried to defer work in DidInsertChildrenOfNode() so that
occurred when layout was clean, but the change was incorrect because:
1. DidInsertChildrenOfNodeWithCleanLayout() wasn't even called!
2. Even if that method is called via DeferTreeUpdate, the loop
   would not iterate as before, because by the time it's called,
   Get(node) will return different results for some nodes. Therefore,
   do the loop right away, and call TextChanged() on the first ancestor
   with an AXObject, as TextChanged() defers its work anyway.

Bug: 1136770
Change-Id: I5ae270317f506bd7ba4fc3cf27b7728b5dca73a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533139
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827066}
parent c6bacb04
......@@ -321,6 +321,12 @@ static bool IsMenuListOption(const Node* node) {
return select->GetLayoutObject();
}
AXObject* AXObjectCacheImpl::GetIfExists(const Node* node) {
AXID node_id = node_object_mapping_.at(node);
DCHECK(!HashTraits<AXID>::IsDeletedValue(node_id));
return node_id ? objects_.at(node_id) : nullptr;
}
// TODO(aleventhal) Remove side effects or rename, e.g. GetUpdated().
AXObject* AXObjectCacheImpl::Get(const Node* node) {
if (!node)
......@@ -1215,23 +1221,12 @@ void AXObjectCacheImpl::DidInsertChildrenOfNode(Node* node) {
// accessibility tree, notify the root of that subtree that its children have
// changed.
DCHECK(node);
DeferTreeUpdate(&AXObjectCacheImpl::ChildrenChangedWithCleanLayout, node);
}
void AXObjectCacheImpl::DidInsertChildrenOfNodeWithCleanLayout(Node* node) {
if (!node)
return;
#if DCHECK_IS_ON()
Document* document = &node->GetDocument();
DCHECK(document->Lifecycle().GetState() >= DocumentLifecycle::kLayoutClean)
<< "Unclean document at lifecycle " << document->Lifecycle().ToString();
#endif // DCHECK_IS_ON()
if (AXObject* obj = Get(node)) {
TextChangedWithCleanLayout(node);
} else {
DidInsertChildrenOfNodeWithCleanLayout(NodeTraversal::Parent(*node));
while (node) {
if (AXObject* obj = GetIfExists(node)) {
TextChanged(node);
return;
}
node = NodeTraversal::Parent(*node);
}
}
......
......@@ -191,11 +191,16 @@ class MODULES_EXPORT AXObjectCacheImpl
AXID GetAXID(Node*) override;
Element* GetElementFromAXID(AXID) override;
// will only return the AXObject if it already exists
// Will only return the AXObject if it already exists.
AXObject* GetIfExists(const Node*);
AXObject* Get(AccessibleNode*);
AXObject* Get(AbstractInlineTextBox*);
// These can actually return a different AXObject* if it's determined that
// the wrong type currently axists (AXNodeObject vs AXLayoutObject).
// TODO(aleventhal) These should not have any side effects.
AXObject* Get(const Node*) override;
AXObject* Get(const LayoutObject*);
AXObject* Get(AbstractInlineTextBox*);
AXObject* FirstAccessibleObjectFromNode(const Node*);
......@@ -218,7 +223,6 @@ class MODULES_EXPORT AXObjectCacheImpl
void DidShowMenuListPopupWithCleanLayout(Node*);
void DidHideMenuListPopupWithCleanLayout(Node*);
void StyleChangedWithCleanLayout(Node*);
void DidInsertChildrenOfNodeWithCleanLayout(Node*);
void HandleScrollPositionChangedWithCleanLayout(Node*);
void HandleValidationMessageVisibilityChangedWithCleanLayout(const Node*);
......
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