[ATK] Defer the emision of AtkObject::children-changed signal after layout is done
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Apr 2019 08:01:01 +0000 (08:01 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Apr 2019 08:01:01 +0000 (08:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187948

Reviewed by Michael Catanzaro.

Source/WebCore:

The signal AtkObject::children-changed is emitted from AXObjectCache::attachWrapper() and
AXObjectCache::detachWrapper(). Both can be called in the middle of a layout, so we need to defer the emission
of the signal after the layout is done, to avoid other atk entry points from being called at that point, since
most of them update the backing store at the beginning.

Fixes: accessibility/children-changed-sends-notification.html

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::performDeferredCacheUpdate): Call platformPerformDeferredCacheUpdate().
* accessibility/AXObjectCache.h:
* accessibility/atk/AXObjectCacheAtk.cpp:
(WebCore::wrapperParent): Helper to get the AtkObject parent of a given WebKitAccessible.
(WebCore::AXObjectCache::detachWrapper): Add wrapper to m_deferredDetachedWrapperList.
(WebCore::AXObjectCache::attachWrapper): Add object to m_deferredAttachedWrapperObjectList.
(WebCore::AXObjectCache::platformPerformDeferredCacheUpdate): Emit AtkObject::children-changed::add for objects
in m_deferredAttachedWrapperObjectList and AtkObject::children-changed::remove for wrappers in m_deferredDetachedWrapperList.
* accessibility/ios/AXObjectCacheIOS.mm:
(WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
* accessibility/mac/AXObjectCacheMac.mm:
(WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
* accessibility/win/AXObjectCacheWin.cpp:
(WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
* accessibility/wpe/AXObjectCacheWPE.cpp:
(WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):

LayoutTests:

Remove expectations of accessibility/children-changed-sends-notification.html that passes now.

* platform/gtk/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244107 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/gtk/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp
Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm
Source/WebCore/accessibility/mac/AXObjectCacheMac.mm
Source/WebCore/accessibility/win/AXObjectCacheWin.cpp
Source/WebCore/accessibility/wpe/AXObjectCacheWPE.cpp

index dbac7b6..a8f1815 100644 (file)
@@ -1,5 +1,16 @@
 2019-04-10  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [ATK] Defer the emision of AtkObject::children-changed signal after layout is done
+        https://bugs.webkit.org/show_bug.cgi?id=187948
+
+        Reviewed by Michael Catanzaro.
+
+        Remove expectations of accessibility/children-changed-sends-notification.html that passes now.
+
+        * platform/gtk/TestExpectations:
+
+2019-04-10  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         [ATK] Test accessibility/insert-children-assert.html is crashing since added in r216980
         https://bugs.webkit.org/show_bug.cgi?id=172281
         <rdar://problem/37030990>
index 1332fad..2ed65e7 100644 (file)
@@ -3212,7 +3212,6 @@ webkit.org/b/160249 fast/shrink-wrap/rect-shrink-wrap.html [ ImageOnlyFailure ]
 webkit.org/b/160251 fast/text/font-kerning.html [ ImageOnlyFailure ]
 
 webkit.org/b/161583 accessibility/auto-fill-types.html [ Failure ]
-webkit.org/b/161584 accessibility/children-changed-sends-notification.html [ Failure Crash ]
 
 webkit.org/b/161587 css3/font-feature-settings-rendering.html [ ImageOnlyFailure ]
 webkit.org/b/161588 css3/font-variant-synthesis-jdaggett.html [ ImageOnlyFailure ]
index 04c00d3..6ef3806 100644 (file)
@@ -1,5 +1,37 @@
 2019-04-10  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [ATK] Defer the emision of AtkObject::children-changed signal after layout is done
+        https://bugs.webkit.org/show_bug.cgi?id=187948
+
+        Reviewed by Michael Catanzaro.
+
+        The signal AtkObject::children-changed is emitted from AXObjectCache::attachWrapper() and
+        AXObjectCache::detachWrapper(). Both can be called in the middle of a layout, so we need to defer the emission
+        of the signal after the layout is done, to avoid other atk entry points from being called at that point, since
+        most of them update the backing store at the beginning.
+
+        Fixes: accessibility/children-changed-sends-notification.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::performDeferredCacheUpdate): Call platformPerformDeferredCacheUpdate().
+        * accessibility/AXObjectCache.h:
+        * accessibility/atk/AXObjectCacheAtk.cpp:
+        (WebCore::wrapperParent): Helper to get the AtkObject parent of a given WebKitAccessible.
+        (WebCore::AXObjectCache::detachWrapper): Add wrapper to m_deferredDetachedWrapperList.
+        (WebCore::AXObjectCache::attachWrapper): Add object to m_deferredAttachedWrapperObjectList.
+        (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate): Emit AtkObject::children-changed::add for objects
+        in m_deferredAttachedWrapperObjectList and AtkObject::children-changed::remove for wrappers in m_deferredDetachedWrapperList.
+        * accessibility/ios/AXObjectCacheIOS.mm:
+        (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
+        * accessibility/mac/AXObjectCacheMac.mm:
+        (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
+        * accessibility/win/AXObjectCacheWin.cpp:
+        (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
+        * accessibility/wpe/AXObjectCacheWPE.cpp:
+        (WebCore::AXObjectCache::platformPerformDeferredCacheUpdate):
+
+2019-04-10  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         [ATK] Test accessibility/insert-children-assert.html is crashing since added in r216980
         https://bugs.webkit.org/show_bug.cgi?id=172281
         <rdar://problem/37030990>
index 11a1348..196bc28 100644 (file)
@@ -2919,6 +2919,8 @@ void AXObjectCache::performDeferredCacheUpdate()
     for (auto& deferredFocusedChangeContext : m_deferredFocusedNodeChange)
         handleFocusedUIElementChanged(deferredFocusedChangeContext.first, deferredFocusedChangeContext.second);
     m_deferredFocusedNodeChange.clear();
+
+    platformPerformDeferredCacheUpdate();
 }
     
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
index 3439fad..6472114 100644 (file)
 #include <wtf/ListHashSet.h>
 #include <wtf/RefPtr.h>
 
+#if PLATFORM(GTK)
+#include <wtf/glib/GRefPtr.h>
+#endif
+
 namespace WebCore {
 
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
@@ -355,6 +359,8 @@ protected:
     void postPlatformNotification(AccessibilityObject*, AXNotification);
     void platformHandleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
 
+    void platformPerformDeferredCacheUpdate();
+
 #if PLATFORM(COCOA)
     void postTextStateChangePlatformNotification(AccessibilityObject*, const AXTextStateChangeIntent&, const VisibleSelection&);
     void postTextStateChangePlatformNotification(AccessibilityObject*, AXTextEditType, const String&, const VisiblePosition&);
@@ -476,6 +482,11 @@ private:
     Vector<std::pair<Node*, Node*>> m_deferredFocusedNodeChange;
     bool m_isSynchronizingSelection { false };
     bool m_performingDeferredCacheUpdate { false };
+
+#if PLATFORM(GTK)
+    ListHashSet<RefPtr<AccessibilityObject>> m_deferredAttachedWrapperObjectList;
+    ListHashSet<GRefPtr<AccessibilityObjectWrapper>> m_deferredDetachedWrapperList;
+#endif
 };
 
 class AXAttributeCacheEnabler
index 505d694..a05ec17 100644 (file)
 
 namespace WebCore {
 
+static AtkObject* wrapperParent(WebKitAccessible* wrapper)
+{
+    // Look for the right object to emit the signal from, but using the implementation
+    // of atk_object_get_parent from AtkObject class (which uses a cached pointer if set)
+    // since the accessibility hierarchy in WebCore will no longer be navigable.
+    gpointer webkitAccessibleClass = g_type_class_peek_parent(WEBKIT_ACCESSIBLE_GET_CLASS(wrapper));
+    gpointer atkObjectClass = g_type_class_peek_parent(webkitAccessibleClass);
+    AtkObject* atkParent = ATK_OBJECT_CLASS(atkObjectClass)->get_parent(ATK_OBJECT(wrapper));
+    // We don't want to emit any signal from an object outside WebKit's world.
+    return WEBKIT_IS_ACCESSIBLE(atkParent) ? atkParent : nullptr;
+}
+
 void AXObjectCache::detachWrapper(AccessibilityObject* obj, AccessibilityDetachmentType detachmentType)
 {
     auto* wrapper = obj->wrapper();
@@ -43,23 +55,8 @@ void AXObjectCache::detachWrapper(AccessibilityObject* obj, AccessibilityDetachm
 
     // If an object is being detached NOT because of the AXObjectCache being destroyed,
     // then it's being removed from the accessibility tree and we should emit a signal.
-    if (detachmentType != AccessibilityDetachmentType::CacheDestroyed) {
-        if (obj->document()) {
-            // Look for the right object to emit the signal from, but using the implementation
-            // of atk_object_get_parent from AtkObject class (which uses a cached pointer if set)
-            // since the accessibility hierarchy in WebCore will no longer be navigable.
-            gpointer webkitAccessibleClass = g_type_class_peek_parent(WEBKIT_ACCESSIBLE_GET_CLASS(wrapper));
-            gpointer atkObjectClass = g_type_class_peek_parent(webkitAccessibleClass);
-            AtkObject* atkParent = ATK_OBJECT_CLASS(atkObjectClass)->get_parent(ATK_OBJECT(wrapper));
-
-            // We don't want to emit any signal from an object outside WebKit's world.
-            if (WEBKIT_IS_ACCESSIBLE(atkParent)) {
-                // The accessibility hierarchy is already invalid, so the parent-children relationships
-                // in the AccessibilityObject tree are not there anymore, so we can't know the offset.
-                g_signal_emit_by_name(atkParent, "children-changed::remove", -1, wrapper);
-            }
-        }
-    }
+    if (detachmentType != AccessibilityDetachmentType::CacheDestroyed && obj->document() && wrapperParent(wrapper))
+        m_deferredDetachedWrapperList.add(wrapper);
 
     webkitAccessibleDetach(WEBKIT_ACCESSIBLE(wrapper));
 }
@@ -87,18 +84,39 @@ void AXObjectCache::attachWrapper(AccessibilityObject* obj)
     if (!obj->renderer())
         return;
 
-    // Don't emit the signal for objects whose parents won't be exposed directly.
-    AccessibilityObject* coreParent = obj->parentObjectUnignored();
-    if (!coreParent || coreParent->accessibilityIsIgnoredByDefault())
-        return;
+    m_deferredAttachedWrapperObjectList.add(obj);
+}
 
-    // Look for the right object to emit the signal from.
-    auto* atkParent = coreParent->wrapper();
-    if (!atkParent)
-        return;
+void AXObjectCache::platformPerformDeferredCacheUpdate()
+{
+    for (auto& coreObject : m_deferredAttachedWrapperObjectList) {
+        auto* wrapper = coreObject->wrapper();
+        if (!wrapper)
+            continue;
+
+        // Don't emit the signal for objects whose parents won't be exposed directly.
+        auto* coreParent = coreObject->parentObjectUnignored();
+        if (!coreParent || coreParent->accessibilityIsIgnoredByDefault())
+            continue;
+
+        // Look for the right object to emit the signal from.
+        auto* atkParent = coreParent->wrapper();
+        if (!atkParent)
+            continue;
+
+        size_t index = coreParent->children(false).find(coreObject);
+        g_signal_emit_by_name(atkParent, "children-changed::add", index != notFound ? index : -1, wrapper);
+    }
+    m_deferredAttachedWrapperObjectList.clear();
 
-    size_t index = coreParent->children(false).find(obj);
-    g_signal_emit_by_name(atkParent, "children-changed::add", index != notFound ? index : -1, wrapper.get());
+    for (auto& wrapper : m_deferredDetachedWrapperList) {
+        if (auto* atkParent = wrapperParent(wrapper.get())) {
+            // The accessibility hierarchy is already invalid, so the parent-children relationships
+            // in the AccessibilityObject tree are not there anymore, so we can't know the offset.
+            g_signal_emit_by_name(atkParent, "children-changed::remove", -1, wrapper.get());
+        }
+    }
+    m_deferredDetachedWrapperList.clear();
 }
 
 static AccessibilityObject* getListObject(AccessibilityObject* object)
index b1c9b02..18752be 100644 (file)
@@ -135,7 +135,11 @@ void AXObjectCache::platformHandleFocusedUIElementChanged(Node*, Node* newNode)
 void AXObjectCache::handleScrolledToAnchor(const Node*)
 {
 }
-    
+
+void AXObjectCache::platformPerformDeferredCacheUpdate()
+{
+}
+
 }
 
 #endif // HAVE(ACCESSIBILITY) && PLATFORM(IOS_FAMILY)
index 1bb5898..5d45e44 100644 (file)
@@ -547,6 +547,10 @@ void AXObjectCache::handleScrolledToAnchor(const Node*)
 {
 }
 
+void AXObjectCache::platformPerformDeferredCacheUpdate()
+{
+}
+
 }
 
 #endif // HAVE(ACCESSIBILITY) && PLATFORM(MAC)
index 546f381..4909a6b 100644 (file)
@@ -183,4 +183,8 @@ void AXObjectCache::platformHandleFocusedUIElementChanged(Node*, Node* newFocuse
     postPlatformNotification(focusedObject, AXFocusedUIElementChanged);
 }
 
+void AXObjectCache::platformPerformDeferredCacheUpdate()
+{
+}
+
 } // namespace WebCore
index 1541594..008acca 100644 (file)
@@ -60,6 +60,11 @@ void AXObjectCache::handleScrolledToAnchor(const Node*)
     notImplemented();
 }
 
+void AXObjectCache::platformPerformDeferredCacheUpdate()
+{
+    notImplemented();
+}
+
 } // namespace WebCore
 
 #endif // HAVE(ACCESSIBILITY)