Diff
Modified: trunk/Source/WebCore/ChangeLog (258970 => 258971)
--- trunk/Source/WebCore/ChangeLog 2020-03-25 03:31:27 UTC (rev 258970)
+++ trunk/Source/WebCore/ChangeLog 2020-03-25 03:48:33 UTC (rev 258971)
@@ -1,3 +1,42 @@
+2020-03-24 Andres Gonzalez <[email protected]>
+
+ Avoid multiple unnecessary updates of the IsolatedTree.
+ https://bugs.webkit.org/show_bug.cgi?id=209409
+
+ Reviewed by Chris Fleizach.
+
+ AXObjectCache::notificationPostTimerFired was updating the isolated tree
+ in every single notification, causing a big performance hit.
+ This change filters out repeated notifications for the same node, thus
+ reducing significantly the number of times the isolated tree is updated.
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::notificationPostTimerFired):
+ (WebCore::AXObjectCache::postNotification):
+ (WebCore::AXObjectCache::postTextStateChangeNotification):
+ (WebCore::AXObjectCache::generateIsolatedTree):
+ (WebCore::AXObjectCache::updateIsolatedTree):
+ (WebCore::appendIfNotContainsMatching): Helper function that might be
+ added to WTF::Vector.
+ (WebCore::createIsolatedTreeHierarchy): Became AXIsolatedTree::createSubtree
+ * accessibility/AXObjectCache.h:
+ * accessibility/AccessibilityObjectInterface.h:
+ (WebCore::AXCoreObject::childrenIDs):
+ * accessibility/isolatedtree/AXIsolatedTree.cpp:
+ (WebCore::AXIsolatedTree::removeTreeForPageID):
+ (WebCore::AXIsolatedTree::generateSubtree):
+ (WebCore::AXIsolatedTree::createSubtree):
+ (WebCore::AXIsolatedTree::updateNode): Updates only the given node.
+ (WebCore::AXIsolatedTree::updateSubtree): Recreates the entire subtree.
+ (WebCore::AXIsolatedTree::updateChildren): Updates the associated object,
+ recreating only the children that are added and removing the ones that
+ are no longer present in the AX tree.
+ (WebCore::AXIsolatedTree::removeNode):
+ (WebCore::AXIsolatedTree::removeSubtree):
+ (WebCore::AXIsolatedTree::appendNodeChanges):
+ (WebCore::AXIsolatedTree::applyPendingChanges):
+ * accessibility/isolatedtree/AXIsolatedTree.h:
+
2020-03-24 John Wilander <[email protected]>
Build fix for deprecated DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (258970 => 258971)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2020-03-25 03:31:27 UTC (rev 258970)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2020-03-25 03:48:33 UTC (rev 258971)
@@ -1025,7 +1025,7 @@
// In tests, posting notifications has a tendency to immediately queue up other notifications, which can lead to unexpected behavior
// when the notification list is cleared at the end. Instead copy this list at the start.
auto notifications = WTFMove(m_notificationsToPost);
-
+
for (const auto& note : notifications) {
AXCoreObject* obj = note.first.get();
if (!obj->objectID())
@@ -1056,12 +1056,12 @@
if (notification == AXChildrenChanged && obj->parentObjectIfExists() && obj->lastKnownIsIgnoredValue() != obj->accessibilityIsIgnored())
childrenChanged(obj->parentObject());
+ postPlatformNotification(obj, notification);
+ }
+
#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
- updateIsolatedTree(obj, notification);
+ updateIsolatedTree(notifications);
#endif
-
- postPlatformNotification(obj, notification);
- }
}
void AXObjectCache::passwordNotificationPostTimerFired()
@@ -1139,7 +1139,7 @@
m_notificationPostTimer.startOneShot(0_s);
} else {
#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
- updateIsolatedTree(object, notification);
+ updateIsolatedTree(*object, notification);
#endif
postPlatformNotification(object, notification);
@@ -1375,7 +1375,7 @@
}
#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
- updateIsolatedTree(object, AXSelectedTextChanged);
+ updateIsolatedTree(*object, AXSelectedTextChanged);
#endif
postTextStateChangeNotification(object, intent, selection);
@@ -1425,7 +1425,7 @@
}
#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
- updateIsolatedTree(object, AXValueChanged);
+ updateIsolatedTree(*object, AXValueChanged);
#endif
postTextStateChangePlatformNotification(object, type, text, position);
@@ -3083,27 +3083,6 @@
}
#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
-static Ref<AXIsolatedObject> createIsolatedTreeHierarchy(AXCoreObject& object, AXIsolatedTreeID treeID, AXID parentID, bool attachWrapper, Vector<AXIsolatedTree::NodeChange>& nodeChanges)
-{
- auto isolatedObject = AXIsolatedObject::create(object, treeID, parentID);
- if (attachWrapper) {
- isolatedObject->attachPlatformWrapper(object.wrapper());
- // Since this object has already an attached wrapper, set the wrapper
- // in the NodeChange to null so that it is not re-attached.
- nodeChanges.append(AXIsolatedTree::NodeChange(isolatedObject, nullptr));
- } else {
- // Set the wrapper in the NodeChange so that it is set on the AX thread.
- nodeChanges.append(AXIsolatedTree::NodeChange(isolatedObject, object.wrapper()));
- }
-
- for (const auto& child : object.children()) {
- auto staticChild = createIsolatedTreeHierarchy(*child, treeID, isolatedObject->objectID(), attachWrapper, nodeChanges);
- isolatedObject->appendChild(staticChild->objectID());
- }
-
- return isolatedObject;
-}
-
Ref<AXIsolatedTree> AXObjectCache::generateIsolatedTree(PageIdentifier pageID, Document& document)
{
RELEASE_ASSERT(isMainThread());
@@ -3118,12 +3097,8 @@
tree->setAXObjectCache(axObjectCache);
auto* axRoot = axObjectCache->getOrCreate(document.view());
- if (axRoot) {
- Vector<AXIsolatedTree::NodeChange> nodeChanges;
- auto isolatedRoot = createIsolatedTreeHierarchy(*axRoot, tree->treeIdentifier(), InvalidAXID, true, nodeChanges);
- tree->setRootNode(isolatedRoot);
- tree->appendNodeChanges(nodeChanges);
- }
+ if (axRoot)
+ tree->generateSubtree(*axRoot, InvalidAXID, true);
auto* axFocus = axObjectCache->focusedObject(document);
if (axFocus)
@@ -3132,7 +3107,7 @@
return makeRef(*tree);
}
-void AXObjectCache::updateIsolatedTree(AXCoreObject* object, AXNotification notification)
+void AXObjectCache::updateIsolatedTree(AXCoreObject& object, AXNotification notification)
{
if (!m_pageID)
return;
@@ -3143,15 +3118,10 @@
switch (notification) {
case AXCheckedStateChanged:
- case AXChildrenChanged:
case AXSelectedTextChanged:
case AXValueChanged: {
- tree->removeNode(object->objectID());
- auto* parent = object->parentObject();
- AXID parentID = parent ? parent->objectID() : InvalidAXID;
- Vector<AXIsolatedTree::NodeChange> nodeChanges;
- auto isolatedObject = createIsolatedTreeHierarchy(*object, tree->treeIdentifier(), parentID, false, nodeChanges);
- tree->appendNodeChanges(nodeChanges);
+ if (object.objectID() != InvalidAXID)
+ tree->updateNode(object);
break;
}
default:
@@ -3158,8 +3128,61 @@
break;
}
}
+
+// FIXME: should be added to WTF::Vector.
+template<typename T, typename F>
+static bool appendIfNotContainsMatching(Vector<T>& vector, const T& value, F matches)
+{
+ if (vector.findMatching(matches) != notFound)
+ return false;
+ vector.append(value);
+ return true;
+}
+
+void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<AXCoreObject>, AXNotification>>& notifications)
+{
+ if (!m_pageID)
+ return;
+
+ auto tree = AXIsolatedTree::treeForPageID(*m_pageID);
+ if (!tree)
+ return;
+
+ // Filter out multiple notifications for the same object. This avoids
+ // updating the isolated tree multiple times unnecessarily.
+ Vector<std::pair<RefPtr<AXCoreObject>, AXNotification>> filteredNotifications;
+ for (const auto& notification : notifications) {
+ if (!notification.first || notification.first->objectID() == InvalidAXID)
+ continue;
+
+ switch (notification.second) {
+ case AXCheckedStateChanged:
+ case AXSelectedTextChanged:
+ case AXValueChanged: {
+ bool needsUpdate = appendIfNotContainsMatching(filteredNotifications, notification, [¬ification] (const std::pair<RefPtr<AXCoreObject>, AXNotification>& note) {
+ return note.second == notification.second && note.first.get() == notification.first.get();
+ });
+
+ if (needsUpdate)
+ tree->updateNode(*notification.first);
+ break;
+ }
+ case AXChildrenChanged: {
+ bool needsUpdate = appendIfNotContainsMatching(filteredNotifications, notification, [¬ification] (const std::pair<RefPtr<AXCoreObject>, AXNotification>& note) {
+ return note.second == notification.second && note.first.get() == notification.first.get();
+ });
+
+ if (needsUpdate)
+ tree->updateChildren(*notification.first);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+}
#endif
-
+
void AXObjectCache::deferRecomputeIsIgnoredIfNeeded(Element* element)
{
if (!nodeAndRendererAreValid(element))
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (258970 => 258971)
--- trunk/Source/WebCore/accessibility/AXObjectCache.h 2020-03-25 03:31:27 UTC (rev 258970)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h 2020-03-25 03:48:33 UTC (rev 258971)
@@ -360,7 +360,8 @@
void setIsolatedTreeFocusedObject(Node*);
RefPtr<AXIsolatedTree> getOrCreateIsolatedTree() const;
static Ref<AXIsolatedTree> generateIsolatedTree(PageIdentifier, Document&);
- void updateIsolatedTree(AXCoreObject*, AXNotification);
+ void updateIsolatedTree(AXCoreObject&, AXNotification);
+ void updateIsolatedTree(const Vector<std::pair<RefPtr<AXCoreObject>, AXNotification>>&);
static void initializeSecondaryAXThread();
#endif
Modified: trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h (258970 => 258971)
--- trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h 2020-03-25 03:31:27 UTC (rev 258970)
+++ trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h 2020-03-25 03:48:33 UTC (rev 258971)
@@ -904,10 +904,12 @@
virtual void childrenChanged() = 0;
virtual void textChanged() = 0;
virtual void updateAccessibilityRole() = 0;
+
virtual const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true) = 0;
virtual void addChildren() = 0;
virtual void addChild(AXCoreObject*) = 0;
virtual void insertChild(AXCoreObject*, unsigned) = 0;
+ Vector<AXID> childrenIDs();
virtual bool shouldIgnoreAttributeRole() const = 0;
@@ -1201,6 +1203,14 @@
}
#endif
+inline Vector<AXID> AXCoreObject::childrenIDs()
+{
+ Vector<AXID> childrenIDs;
+ for (const auto& child : children())
+ childrenIDs.append(child->objectID());
+ return childrenIDs;
+}
+
namespace Accessibility {
template<typename T, typename F>
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp (258970 => 258971)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp 2020-03-25 03:31:27 UTC (rev 258970)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp 2020-03-25 03:48:33 UTC (rev 258971)
@@ -109,7 +109,7 @@
auto& tree { *optionalTree };
LockHolder treeLocker { tree->m_changeLogLock };
- tree->m_pendingRemovals.append(tree->m_rootNodeID);
+ tree->m_pendingSubtreeRemovals.append(tree->m_rootNodeID);
tree->setAXObjectCache(nullptr);
treeLocker.unlockEarly();
@@ -145,6 +145,102 @@
return result;
}
+void AXIsolatedTree::generateSubtree(AXCoreObject& axObject, AXID parentID, bool attachWrapper)
+{
+ ASSERT(isMainThread());
+ Vector<NodeChange> nodeChanges;
+ auto object = createSubtree(axObject, parentID, attachWrapper, nodeChanges);
+ appendNodeChanges(nodeChanges);
+
+ if (parentID == InvalidAXID)
+ setRootNode(object);
+ // FIXME: else attach the newly created subtree to its parent.
+}
+
+Ref<AXIsolatedObject> AXIsolatedTree::createSubtree(AXCoreObject& axObject, AXID parentID, bool attachWrapper, Vector<NodeChange>& nodeChanges)
+{
+ ASSERT(isMainThread());
+ auto object = AXIsolatedObject::create(axObject, m_treeID, parentID);
+ if (attachWrapper) {
+ object->attachPlatformWrapper(axObject.wrapper());
+ // Since this object has already an attached wrapper, set the wrapper
+ // in the NodeChange to null so that it is not re-attached.
+ nodeChanges.append(NodeChange(object, nullptr));
+ } else {
+ // Set the wrapper in the NodeChange so that it is set on the AX thread.
+ nodeChanges.append(NodeChange(object, axObject.wrapper()));
+ }
+
+ for (const auto& axChild : axObject.children()) {
+ auto child = createSubtree(*axChild, object->objectID(), attachWrapper, nodeChanges);
+ object->appendChild(child->objectID());
+ }
+
+ return object;
+}
+
+void AXIsolatedTree::updateNode(AXCoreObject& axObject)
+{
+ ASSERT(isMainThread());
+ AXID axID = axObject.objectID();
+ auto* axParent = axObject.parentObject();
+ AXID parentID = axParent ? axParent->objectID() : InvalidAXID;
+
+ if (auto object = nodeForID(axID)) {
+ ASSERT(object->objectID() == axID);
+ auto newObject = AXIsolatedObject::create(axObject, m_treeID, parentID);
+
+ LockHolder locker { m_changeLogLock };
+ // The new object should have the same children as the old one.
+ newObject->m_childrenIDs = object->m_childrenIDs;
+ // Remove the old object and set the new one to be updated on the AX thread.
+ m_pendingNodeRemovals.append(axID);
+ m_pendingAppends.append(NodeChange(newObject, axObject.wrapper()));
+ }
+}
+
+void AXIsolatedTree::updateSubtree(AXCoreObject& axObject)
+{
+ ASSERT(isMainThread());
+ removeSubtree(axObject.objectID());
+ auto* axParent = axObject.parentObject();
+ AXID parentID = axParent ? axParent->objectID() : InvalidAXID;
+ generateSubtree(axObject, parentID, false);
+}
+
+void AXIsolatedTree::updateChildren(AXCoreObject& axObject)
+{
+ ASSERT(isMainThread());
+ AXID axObjectID = axObject.objectID();
+ auto object = nodeForID(axObjectID);
+ if (!object)
+ return; // nothing to update.
+
+ const auto& axChildren = axObject.children();
+ auto axChildrenIDs = axObject.childrenIDs();
+
+ LockHolder locker { m_changeLogLock };
+ auto removals = object->m_childrenIDs;
+ // Make the children IDs of the isolated object to be the same as the AXObject's.
+ object->m_childrenIDs = axChildrenIDs;
+ locker.unlockEarly();
+
+ for (size_t i = 0; i < axChildrenIDs.size(); ++i) {
+ size_t index = removals.find(axChildrenIDs[i]);
+ if (index != notFound)
+ removals.remove(index);
+ else {
+ // This is a new child, add it to the tree.
+ generateSubtree(*axChildren[i], axObjectID, false);
+ }
+ }
+
+ // What is left in removals are the IDs that are no longer children of
+ // axObject. Thus, remove them from the tree.
+ for (const AXID& childID : removals)
+ removeSubtree(childID);
+}
+
RefPtr<AXIsolatedObject> AXIsolatedTree::focusedUIElement()
{
return nodeForID(m_focusedNodeID);
@@ -188,20 +284,25 @@
LockHolder locker { m_changeLogLock };
m_pendingFocusedNodeID = axID;
}
-
+
void AXIsolatedTree::removeNode(AXID axID)
{
LockHolder locker { m_changeLogLock };
- m_pendingRemovals.append(axID);
+ m_pendingNodeRemovals.append(axID);
}
-void AXIsolatedTree::appendNodeChanges(const Vector<NodeChange>& changes)
+void AXIsolatedTree::removeSubtree(AXID axID)
{
LockHolder locker { m_changeLogLock };
- for (const auto& nodeChange : changes)
- m_pendingAppends.append(nodeChange);
+ m_pendingSubtreeRemovals.append(axID);
}
+void AXIsolatedTree::appendNodeChanges(const Vector<NodeChange>& changes)
+{
+ ASSERT(isMainThread());
+ m_pendingAppends.appendVector(changes);
+}
+
void AXIsolatedTree::applyPendingChanges()
{
RELEASE_ASSERT(!isMainThread());
@@ -209,14 +310,23 @@
m_focusedNodeID = m_pendingFocusedNodeID;
- while (m_pendingRemovals.size()) {
- auto axID = m_pendingRemovals.takeLast();
+ while (m_pendingNodeRemovals.size()) {
+ auto axID = m_pendingNodeRemovals.takeLast();
if (axID == InvalidAXID)
continue;
+ if (auto object = nodeForID(axID))
+ object->detach(AccessibilityDetachmentType::ElementDestroyed);
+ }
+
+ while (m_pendingSubtreeRemovals.size()) {
+ auto axID = m_pendingSubtreeRemovals.takeLast();
+ if (axID == InvalidAXID)
+ continue;
+
if (auto object = nodeForID(axID)) {
object->detach(AccessibilityDetachmentType::ElementDestroyed);
- m_pendingRemovals.appendVector(object->m_childrenIDs);
+ m_pendingSubtreeRemovals.appendVector(object->m_childrenIDs);
}
}
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h (258970 => 258971)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h 2020-03-25 03:31:27 UTC (rev 258970)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h 2020-03-25 03:48:33 UTC (rev 258971)
@@ -51,13 +51,13 @@
static Ref<AXIsolatedTree> createTreeForPageID(PageIdentifier);
static void removeTreeForPageID(PageIdentifier);
- WEBCORE_EXPORT static RefPtr<AXIsolatedTree> treeForPageID(PageIdentifier);
- WEBCORE_EXPORT static RefPtr<AXIsolatedTree> treeForID(AXIsolatedTreeID);
+ static RefPtr<AXIsolatedTree> treeForPageID(PageIdentifier);
+ static RefPtr<AXIsolatedTree> treeForID(AXIsolatedTreeID);
AXObjectCache* axObjectCache() const { return m_axObjectCache; }
void setAXObjectCache(AXObjectCache* axObjectCache) { m_axObjectCache = axObjectCache; }
- WEBCORE_EXPORT RefPtr<AXIsolatedObject> rootNode();
- WEBCORE_EXPORT RefPtr<AXIsolatedObject> focusedUIElement();
+ RefPtr<AXIsolatedObject> rootNode();
+ RefPtr<AXIsolatedObject> focusedUIElement();
RefPtr<AXIsolatedObject> nodeForID(AXID) const;
static RefPtr<AXIsolatedObject> nodeInTreeForID(AXIsolatedTreeID, AXID);
Vector<RefPtr<AXCoreObject>> objectsForIDs(Vector<AXID>) const;
@@ -69,10 +69,15 @@
NodeChange(const NodeChange&);
};
- // Call on main thread
- void appendNodeChanges(const Vector<NodeChange>&);
+ void generateSubtree(AXCoreObject&, AXID parentID, bool attachWrapper);
+ void updateNode(AXCoreObject&);
+ void updateSubtree(AXCoreObject&);
+ void updateChildren(AXCoreObject&);
+
+ // Removes the given node leaving all descendants alone.
+ void removeNode(AXID);
// Removes the given node and all its descendants.
- void removeNode(AXID);
+ void removeSubtree(AXID);
// Both setRootNode and setFocusedNode must be called only during the
// generation of the IsolatedTree.
@@ -94,6 +99,11 @@
static HashMap<AXIsolatedTreeID, Ref<AXIsolatedTree>>& treeIDCache();
static HashMap<PageIdentifier, Ref<AXIsolatedTree>>& treePageCache();
+ // Call on main thread
+ Ref<AXIsolatedObject> createSubtree(AXCoreObject&, AXID parentID, bool attachWrapper, Vector<NodeChange>&);
+ // Queues all pending additions to the tree as the result of a subtree generation.
+ void appendNodeChanges(const Vector<NodeChange>&);
+
AXObjectCache* m_axObjectCache { nullptr };
// Only access on AX thread requesting data.
@@ -100,8 +110,9 @@
HashMap<AXID, Ref<AXIsolatedObject>> m_readerThreadNodeMap;
// Written to by main thread under lock, accessed and applied by AX thread.
- Vector<NodeChange> m_pendingAppends;
- Vector<AXID> m_pendingRemovals;
+ Vector<NodeChange> m_pendingAppends; // Nodes to be added to the tree and platform-wrapped.
+ Vector<AXID> m_pendingNodeRemovals; // Nodes to be removed from the tree.
+ Vector<AXID> m_pendingSubtreeRemovals; // Nodes whose subtrees are to be removed from the tree.
AXID m_pendingFocusedNodeID { InvalidAXID };
Lock m_changeLogLock;
