Log Message
[GTK][a11y] Defer the emission of AddAccessible signal with ATSPI https://bugs.webkit.org/show_bug.cgi?id=234740
Reviewed by Adrian Perez de Castro. In case of node added and removed quickly, we just avoid the emission of the signal. * accessibility/atspi/AXObjectCacheAtspi.cpp: (WebCore::AXObjectCache::attachWrapper): Notify the root about its new child earlier to ensure the isolated tree is created as soon as possible. (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate): Remove the root case from here. * accessibility/atspi/AccessibilityAtspi.cpp: (WebCore::AccessibilityAtspi::parentChanged): Ensure object is cached before emitting the signal. (WebCore::AccessibilityAtspi::childrenChanged): Ditto. (WebCore::AccessibilityAtspi::stateChanged): Ditto. (WebCore::AccessibilityAtspi::textChanged): Ditto. (WebCore::AccessibilityAtspi::textAttributesChanged): Ditto. (WebCore::AccessibilityAtspi::textCaretMoved): Ditto. (WebCore::AccessibilityAtspi::textSelectionChanged): Ditto. (WebCore::AccessibilityAtspi::valueChanged): Ditto. (WebCore::AccessibilityAtspi::selectionChanged): Ditto. (WebCore::AccessibilityAtspi::loadEvent): Ditto. (WebCore::AccessibilityAtspi::addToCacheIfNeeded): Helper to add an object to the cache and emit AddAccessible signal. (WebCore::AccessibilityAtspi::cacheUpdateTimerFired): Update the cache with pending adds. (WebCore::AccessibilityAtspi::scheduleCacheUpdate): Create the update cache timer if needed and schedule an update if there isn't a pending one already. (WebCore::AccessibilityAtspi::addAccessible): Add object to the list and schedule a cache update. (WebCore::AccessibilityAtspi::removeAccessible): Return early if the object is in the update cache list. * accessibility/atspi/AccessibilityAtspi.h: * accessibility/atspi/AccessibilityObjectAtspi.cpp: (WebCore::AccessibilityObjectAtspi::registerObject): Remove the call to AccessibilityRootAtspi::child() when an object is registered before being attached to the isolated object. * accessibility/atspi/AccessibilityRootAtspi.cpp: (WebCore::AccessibilityRootAtspi::child const): Set m_isInChild. (WebCore::AccessibilityRootAtspi::childAdded): Return early if the given child is not the current one or called from child(). * accessibility/atspi/AccessibilityRootAtspi.h:
Modified Paths
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/accessibility/atspi/AXObjectCacheAtspi.cpp
- trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp
- trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.h
- trunk/Source/WebCore/accessibility/atspi/AccessibilityObjectAtspi.cpp
- trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.cpp
- trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.h
Diff
Modified: trunk/Source/WebCore/ChangeLog (287917 => 287918)
--- trunk/Source/WebCore/ChangeLog 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/ChangeLog 2022-01-12 12:02:25 UTC (rev 287918)
@@ -1,3 +1,43 @@
+2022-01-12 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [GTK][a11y] Defer the emission of AddAccessible signal with ATSPI
+ https://bugs.webkit.org/show_bug.cgi?id=234740
+
+ Reviewed by Adrian Perez de Castro.
+
+ In case of node added and removed quickly, we just avoid the emission of the signal.
+
+ * accessibility/atspi/AXObjectCacheAtspi.cpp:
+ (WebCore::AXObjectCache::attachWrapper): Notify the root about its new child earlier to ensure the isolated tree
+ is created as soon as possible.
+ (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate): Remove the root case from here.
+ * accessibility/atspi/AccessibilityAtspi.cpp:
+ (WebCore::AccessibilityAtspi::parentChanged): Ensure object is cached before emitting the signal.
+ (WebCore::AccessibilityAtspi::childrenChanged): Ditto.
+ (WebCore::AccessibilityAtspi::stateChanged): Ditto.
+ (WebCore::AccessibilityAtspi::textChanged): Ditto.
+ (WebCore::AccessibilityAtspi::textAttributesChanged): Ditto.
+ (WebCore::AccessibilityAtspi::textCaretMoved): Ditto.
+ (WebCore::AccessibilityAtspi::textSelectionChanged): Ditto.
+ (WebCore::AccessibilityAtspi::valueChanged): Ditto.
+ (WebCore::AccessibilityAtspi::selectionChanged): Ditto.
+ (WebCore::AccessibilityAtspi::loadEvent): Ditto.
+ (WebCore::AccessibilityAtspi::addToCacheIfNeeded): Helper to add an object to the cache and emit AddAccessible signal.
+ (WebCore::AccessibilityAtspi::cacheUpdateTimerFired): Update the cache with pending adds.
+ (WebCore::AccessibilityAtspi::scheduleCacheUpdate): Create the update cache timer if needed and schedule an
+ update if there isn't a pending one already.
+ (WebCore::AccessibilityAtspi::addAccessible): Add object to the list and schedule a cache update.
+ (WebCore::AccessibilityAtspi::removeAccessible): Return early if the object is in the update cache list.
+ * accessibility/atspi/AccessibilityAtspi.h:
+ * accessibility/atspi/AccessibilityObjectAtspi.cpp:
+ (WebCore::AccessibilityObjectAtspi::registerObject): Remove the call to AccessibilityRootAtspi::child() when an
+ object is registered before being attached to the isolated object.
+ * accessibility/atspi/AccessibilityRootAtspi.cpp:
+ (WebCore::AccessibilityRootAtspi::child const): Set m_isInChild.
+ (WebCore::AccessibilityRootAtspi::childAdded): Return early if the given child is not the current one or called
+ from child().
+ * accessibility/atspi/AccessibilityRootAtspi.h:
+
2022-01-12 Martin Robinson <mrobin...@webkit.org>
Interpolation during animation of two empty transform lists should always yield "none"
Modified: trunk/Source/WebCore/accessibility/atspi/AXObjectCacheAtspi.cpp (287917 => 287918)
--- trunk/Source/WebCore/accessibility/atspi/AXObjectCacheAtspi.cpp 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/accessibility/atspi/AXObjectCacheAtspi.cpp 2022-01-12 12:02:25 UTC (rev 287918)
@@ -35,10 +35,18 @@
void AXObjectCache::attachWrapper(AXCoreObject* axObject)
{
- auto wrapper = AccessibilityObjectAtspi::create(axObject, document().page()->accessibilityRootObject());
+ auto* atspiRoot = document().page()->accessibilityRootObject();
+ auto wrapper = AccessibilityObjectAtspi::create(axObject, atspiRoot);
axObject->setWrapper(wrapper.ptr());
+ // Handle the root earlier here, since we know it only has one child.
+ if (!axObject->parentObjectUnignored() && axObject->isScrollView() && axObject->scrollView() == document().view() && atspiRoot) {
+ wrapper->setParent(nullptr); // nullptr means root.
+ return;
+ }
+
m_deferredParentChangedList.add(axObject);
+ m_performCacheUpdateTimer.startOneShot(0_s);
}
void AXObjectCache::platformPerformDeferredCacheUpdate()
@@ -49,11 +57,8 @@
return;
auto* axParent = axObject.parentObjectUnignored();
- if (!axParent) {
- if (axObject.isScrollView() && axObject.scrollView() == document().view())
- wrapper->setParent(nullptr); // nullptr parent means root.
+ if (!axParent)
return;
- }
if (auto* axParentWrapper = axParent->wrapper())
wrapper->setParent(axParentWrapper);
Modified: trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp (287917 => 287918)
--- trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp 2022-01-12 12:02:25 UTC (rev 287918)
@@ -27,7 +27,6 @@
#include <glib/gi18n-lib.h>
#include <wtf/MainThread.h>
#include <wtf/NeverDestroyed.h>
-#include <wtf/SetForScope.h>
#include <wtf/SortedArrayMap.h>
#include <wtf/UUID.h>
@@ -357,6 +356,8 @@
if (atspiObject->registerObject())
return;
+ addToCacheIfPending(atspiObject.get());
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "PropertyChange",
g_variant_new("(siiva{sv})", "accessible-parent", 0, 0, atspiObject->parentReference(), nullptr), nullptr);
});
@@ -388,6 +389,9 @@
if (!atspiObject->isTreeRegistered())
return;
+ addToCacheIfPending(atspiObject.get());
+ addToCacheIfPending(child.get());
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "ChildrenChanged",
g_variant_new("(siiv(so))", change == ChildrenChanged::Added ? "add" : "remove", child->indexInParentForChildrenChanged(change),
0, g_variant_new("(so)", uniqueName(), child->path().utf8().data()), uniqueName(), atspiObject->path().utf8().data()), nullptr);
@@ -402,6 +406,8 @@
if (!m_connection)
return;
+ addToCacheIfPending(child.get());
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, rootObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "ChildrenChanged",
g_variant_new("(siiv(so))", change == ChildrenChanged::Added ? "add" : "remove", 0,
0, g_variant_new("(so)", uniqueName(), child->path().utf8().data()), uniqueName(), rootObject->path().utf8().data()), nullptr);
@@ -423,6 +429,8 @@
if (!shouldEmitSignal("Object", "StateChanged", name.data()))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "StateChanged",
g_variant_new("(siiva{sv})", name.data(), value, 0, g_variant_new_string("0"), nullptr), nullptr);
});
@@ -443,6 +451,8 @@
if (!shouldEmitSignal("Object", "TextChanged", changeType.data()))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "TextChanged",
g_variant_new("(siiva{sv})", changeType.data(), offset, length, g_variant_new_string(text.data()), nullptr), nullptr);
});
@@ -458,6 +468,8 @@
if (!shouldEmitSignal("Object", "TextAttributesChanged"))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "TextAttributesChanged",
g_variant_new("(siiva{sv})", "", 0, 0, g_variant_new_string(""), nullptr), nullptr);
});
@@ -478,6 +490,8 @@
if (!shouldEmitSignal("Object", "TextCaretMoved"))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "TextCaretMoved",
g_variant_new("(siiva{sv})", "", caretOffset, 0, g_variant_new_string(""), nullptr), nullptr);
});
@@ -493,6 +507,8 @@
if (!shouldEmitSignal("Object", "TextSelectionChanged"))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "TextSelectionChanged",
g_variant_new("(siiva{sv})", "", 0, 0, g_variant_new_string(""), nullptr), nullptr);
});
@@ -513,6 +529,8 @@
if (!shouldEmitSignal("Object", "PropertyChange", "accessible-value"))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "PropertyChange",
g_variant_new("(siiva{sv})", "accessible-value", 0, 0, g_variant_new_double(value), nullptr), nullptr);
});
@@ -533,6 +551,8 @@
if (!shouldEmitSignal("Object", "SelectionChanged"))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Object", "SelectionChanged",
g_variant_new("(siiva{sv})", "", 0, 0, g_variant_new_string(""), nullptr), nullptr);
});
@@ -553,6 +573,8 @@
if (!shouldEmitSignal("Document", event.data()))
return;
+ addToCacheIfPending(atspiObject);
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, atspiObject->path().utf8().data(), "org.a11y.atspi.Event.Document", event.data(),
g_variant_new("(siiva{sv})", "", 0, 0, g_variant_new_string(""), nullptr), nullptr);
});
@@ -706,7 +728,6 @@
RELEASE_ASSERT(!isMainThread());
if (!g_strcmp0(methodName, "GetItems")) {
auto& atspi = *static_cast<AccessibilityAtspi*>(userData);
- SetForScope<bool> inGetItems(atspi.m_inGetItems, true);
GVariantBuilder builder = G_VARIANT_BUILDER_INIT(G_VARIANT_TYPE("(" GET_ITEMS_SIGNATURE ")"));
g_variant_builder_open(&builder, G_VARIANT_TYPE(GET_ITEMS_SIGNATURE));
for (auto* rootObject : atspi.m_rootObjects.keys()) {
@@ -728,6 +749,7 @@
wrapper->serialize(&builder);
g_variant_builder_close(&builder);
}
+
g_variant_builder_close(&builder);
g_dbus_method_invocation_return_value(invocation, g_variant_builder_end(&builder));
}
@@ -752,10 +774,11 @@
m_cacheID = g_dbus_connection_register_object(m_connection.get(), "/org/a11y/atspi/cache", const_cast<GDBusInterfaceInfo*>(&webkit_cache_interface), &s_cacheFunctions, this, nullptr, nullptr);
}
-void AccessibilityAtspi::addAccessible(AccessibilityObjectAtspi& atspiObject)
+void AccessibilityAtspi::addToCacheIfNeeded(AccessibilityObjectAtspi& atspiObject)
{
RELEASE_ASSERT(!isMainThread());
- if (!m_connection)
+ atspiObject.updateBackingStore();
+ if (atspiObject.isDefunct())
return;
auto addResult = m_cache.add(atspiObject.path(), &atspiObject);
@@ -762,9 +785,6 @@
if (!addResult.isNewEntry)
return;
- if (m_inGetItems)
- return;
-
GVariantBuilder builder = G_VARIANT_BUILDER_INIT(G_VARIANT_TYPE("(" ITEM_SIGNATURE ")"));
atspiObject.serialize(&builder);
g_dbus_connection_emit_signal(m_connection.get(), nullptr, "/org/a11y/atspi/cache", "org.a11y.atspi.Cache", "AddAccessible",
@@ -771,11 +791,52 @@
g_variant_new("(@(" ITEM_SIGNATURE "))", g_variant_builder_end(&builder)), nullptr);
}
+void AccessibilityAtspi::addToCacheIfPending(AccessibilityObjectAtspi& atspiObject)
+{
+ if (!m_cacheUpdateList.remove(&atspiObject))
+ return;
+
+ addToCacheIfNeeded(atspiObject);
+ if (m_cacheUpdateTimer && m_cacheUpdateList.isEmpty())
+ m_cacheUpdateTimer = nullptr;
+}
+
+void AccessibilityAtspi::cacheUpdateTimerFired()
+{
+ while (!m_cacheUpdateList.isEmpty())
+ addToCacheIfNeeded(*m_cacheUpdateList.takeFirst());
+}
+
+void AccessibilityAtspi::scheduleCacheUpdate()
+{
+ RELEASE_ASSERT(!isMainThread());
+ if (!m_cacheUpdateTimer)
+ m_cacheUpdateTimer = makeUnique<RunLoop::Timer<AccessibilityAtspi>>(RunLoop::current(), this, &AccessibilityAtspi::cacheUpdateTimerFired);
+
+ if (!m_cacheUpdateTimer->isActive())
+ m_cacheUpdateTimer->startOneShot(0_s);
+}
+
+void AccessibilityAtspi::addAccessible(AccessibilityObjectAtspi& atspiObject)
+{
+ RELEASE_ASSERT(!isMainThread());
+ if (!m_connection)
+ return;
+
+ m_cacheUpdateList.add(&atspiObject);
+ scheduleCacheUpdate();
+}
+
void AccessibilityAtspi::removeAccessible(AccessibilityObjectAtspi& atspiObject)
{
RELEASE_ASSERT(!isMainThread());
+ if (m_cacheUpdateList.remove(&atspiObject))
+ return;
+
const auto& path = atspiObject.path();
- m_cache.remove(path);
+ if (!m_cache.remove(path))
+ return;
+
g_dbus_connection_emit_signal(m_connection.get(), nullptr, "/org/a11y/atspi/cache", "org.a11y.atspi.Cache", "RemoveAccessible",
g_variant_new("((so))", uniqueName(), path.utf8().data()), nullptr);
}
Modified: trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.h (287917 => 287918)
--- trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.h 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/accessibility/atspi/AccessibilityAtspi.h 2022-01-12 12:02:25 UTC (rev 287918)
@@ -23,6 +23,7 @@
#include <wtf/CompletionHandler.h>
#include <wtf/FastMalloc.h>
#include <wtf/HashMap.h>
+#include <wtf/ListHashSet.h>
#include <wtf/Vector.h>
#include <wtf/WorkQueue.h>
#include <wtf/glib/GRefPtr.h>
@@ -99,7 +100,11 @@
void removeEventListener(const char* dbusName, const char* eventName);
void ensureCache();
+ void addToCacheIfNeeded(AccessibilityObjectAtspi&);
+ void addToCacheIfPending(AccessibilityObjectAtspi&);
void removeAccessible(AccessibilityObjectAtspi&);
+ void scheduleCacheUpdate();
+ void cacheUpdateTimerFired();
bool shouldEmitSignal(const char* interface, const char* name, const char* detail = "");
@@ -124,7 +129,8 @@
HashMap<AccessibilityObjectAtspi*, Vector<unsigned, 20>> m_atspiHyperlinks;
unsigned m_cacheID { 0 };
HashMap<String, AccessibilityObjectAtspi*> m_cache;
- bool m_inGetItems { false };
+ ListHashSet<RefPtr<AccessibilityObjectAtspi>> m_cacheUpdateList;
+ std::unique_ptr<RunLoop::Timer<AccessibilityAtspi>> m_cacheUpdateTimer;
#if ENABLE(DEVELOPER_MODE)
HashMap<void*, NotificationObserver> m_notificationObservers;
#endif
Modified: trunk/Source/WebCore/accessibility/atspi/AccessibilityObjectAtspi.cpp (287917 => 287918)
--- trunk/Source/WebCore/accessibility/atspi/AccessibilityObjectAtspi.cpp 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/accessibility/atspi/AccessibilityObjectAtspi.cpp 2022-01-12 12:02:25 UTC (rev 287918)
@@ -534,18 +534,7 @@
interfaces.append({ const_cast<GDBusInterfaceInfo*>(&webkit_table_interface), &s_tableFunctions });
if (m_interfaces.contains(Interface::TableCell))
interfaces.append({ const_cast<GDBusInterfaceInfo*>(&webkit_table_cell_interface), &s_tableCellFunctions });
- if (!m_axObject) {
- // Isolated tree hasn't been created yet, call AccessibilityRootAtspi::child()
- // to create it before registering the object.
- Accessibility::performFunctionOnMainThread([this] {
- if (!m_coreObject)
- return;
- if (auto* atspiRoot = root())
- atspiRoot->child();
- });
- }
-
m_path = AccessibilityAtspi::singleton().registerObject(*this, WTFMove(interfaces));
AccessibilityAtspi::singleton().addAccessible(*this);
Modified: trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.cpp (287917 => 287918)
--- trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.cpp 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.cpp 2022-01-12 12:02:25 UTC (rev 287918)
@@ -31,6 +31,7 @@
#include "Page.h"
#include <glib/gi18n-lib.h>
#include <wtf/MainThread.h>
+#include <wtf/SetForScope.h>
namespace WebCore {
@@ -256,6 +257,8 @@
if (!frame.document())
return nullptr;
+ SetForScope<bool> inChild(m_inChild, true);
+
AXObjectCache::enableAccessibility();
AXObjectCache* cache = frame.document()->axObjectCache();
if (!cache)
@@ -267,6 +270,13 @@
void AccessibilityRootAtspi::childAdded(AccessibilityObjectAtspi& child)
{
+ // Don't emit children changed when called from child() because that means the tree is being populated.
+ if (m_inChild || !m_isTreeRegistered.load())
+ return;
+
+ if (this->child() != &child)
+ return;
+
AccessibilityAtspi::singleton().childrenChanged(*this, child, AccessibilityAtspi::ChildrenChanged::Added);
}
Modified: trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.h (287917 => 287918)
--- trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.h 2022-01-12 11:40:15 UTC (rev 287917)
+++ trunk/Source/WebCore/accessibility/atspi/AccessibilityRootAtspi.h 2022-01-12 12:02:25 UTC (rev 287918)
@@ -70,6 +70,7 @@
String m_parentUniqueName;
String m_parentPath;
Atomic<bool> m_isTreeRegistered { false };
+ mutable bool m_inChild { false };
};
} // namespace WebCore
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
