Title: [228745] releases/WebKitGTK/webkit-2.20
Revision
228745
Author
carlo...@webkit.org
Date
2018-02-19 23:45:39 -0800 (Mon, 19 Feb 2018)

Log Message

Merge r228417 - AX: defer focusedUIElement notifications
https://bugs.webkit.org/show_bug.cgi?id=182643
<rdar://problem/37394310>

Reviewed by Zalan Bujtas.

Source/WebCore:

Deferring focus changes for accessibility has a number of benefits.
    1) Reduces the chance of calling into layout during layout.
    2) Coalesces multiple focus notifications that would be needlessly sent.
    3) Improves performance by not calling out to the accessibility notification machinery during layout.

In this patch, I also started making more AXObjectCache calls private. This will reduce the chance that clients
will call into AXObjectCache during unexpected times.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::deferFocusedUIElementChangeIfNeeded):
(WebCore::conditionallyAddNodeToFilterList):
(WebCore::filterVectorPairForRemoval):
(WebCore::filterMapForRemoval):
(WebCore::filterListForRemoval):
(WebCore::AXObjectCache::prepareForDocumentDestruction):
(WebCore::AXObjectCache::performDeferredCacheUpdate):
* accessibility/AXObjectCache.h:
* dom/Document.cpp:
(WebCore::Document::setFocusedElement):

LayoutTests:

* accessibility/mac/aria-menu-item-selected-notification.html:
     Rewrite test to accomodate that focus changes happen asynchronously.
* accessibility/mac/selection-notification-focus-change-expected.txt:
* platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt:
     The order of notifications is different now that focus changes happen later.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog	2018-02-20 07:45:39 UTC (rev 228745)
@@ -1,3 +1,17 @@
+2018-02-12  Chris Fleizach  <cfleiz...@apple.com>
+
+        AX: defer focusedUIElement notifications
+        https://bugs.webkit.org/show_bug.cgi?id=182643
+        <rdar://problem/37394310>
+
+        Reviewed by Zalan Bujtas.
+
+        * accessibility/mac/aria-menu-item-selected-notification.html:
+             Rewrite test to accomodate that focus changes happen asynchronously.
+        * accessibility/mac/selection-notification-focus-change-expected.txt:
+        * platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt:
+             The order of notifications is different now that focus changes happen later.
+
 2018-02-12  John Wilander  <wilan...@apple.com>
 
         Resource Load Statistics: Classify resources as prevalent based on redirects to other prevalent resources

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html	2018-02-20 07:45:39 UTC (rev 228745)
@@ -46,14 +46,20 @@
         // Trigger notification through focus.
         document.getElementById("item1").focus();
 
-        // Trigger notification through aria-selected.
-        document.getElementById("item2").setAttribute("aria-selected", "true");
+        setTimeout(function() {
+            // Trigger notification through aria-selected.
+            document.getElementById("item2").setAttribute("aria-selected", "true");
 
-        // Ensure we don't get a notification when aria-selected is false.
-        document.getElementById("item2").setAttribute("aria-selected", "false");
+            setTimeout(function() {
+                // Ensure we don't get a notification when aria-selected is false.
+                document.getElementById("item2").setAttribute("aria-selected", "false");
 
-        // Trigger another notification through focus to ensure we don't
-        document.getElementById("item3").focus();
+                setTimeout(function() {
+                    // Trigger another notification through focus to ensure we don't
+                    document.getElementById("item3").focus();
+                }, 1);
+            }, 1);
+        }, 1);
     }
 
 </script>

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt	2018-02-20 07:45:39 UTC (rev 228745)
@@ -16,9 +16,9 @@
 Received AXFocusChanged
 
 eventSender.keyDown(tabCharacter)
-Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
+Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
 PASS successfullyParsed is true

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt	2018-02-20 07:45:39 UTC (rev 228745)
@@ -7,9 +7,9 @@
 eventSender.keyDown(tabCharacter);
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
-Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
+Received AXFocusChanged
 
 PASS accessibilityController.accessibleElementById("1").isFocusable is true
 accessibilityController.accessibleElementById("1").takeFocus()
@@ -16,9 +16,9 @@
 Received AXFocusChanged
 
 eventSender.keyDown(tabCharacter)
-Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
+Received AXFocusChanged
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-02-20 07:45:39 UTC (rev 228745)
@@ -1,3 +1,31 @@
+2018-02-12  Chris Fleizach  <cfleiz...@apple.com>
+
+        AX: defer focusedUIElement notifications
+        https://bugs.webkit.org/show_bug.cgi?id=182643
+        <rdar://problem/37394310>
+
+        Reviewed by Zalan Bujtas.
+
+        Deferring focus changes for accessibility has a number of benefits.
+            1) Reduces the chance of calling into layout during layout.
+            2) Coalesces multiple focus notifications that would be needlessly sent.
+            3) Improves performance by not calling out to the accessibility notification machinery during layout.
+
+        In this patch, I also started making more AXObjectCache calls private. This will reduce the chance that clients
+        will call into AXObjectCache during unexpected times.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::deferFocusedUIElementChangeIfNeeded):
+        (WebCore::conditionallyAddNodeToFilterList):
+        (WebCore::filterVectorPairForRemoval):
+        (WebCore::filterMapForRemoval):
+        (WebCore::filterListForRemoval):
+        (WebCore::AXObjectCache::prepareForDocumentDestruction):
+        (WebCore::AXObjectCache::performDeferredCacheUpdate):
+        * accessibility/AXObjectCache.h:
+        * dom/Document.cpp:
+        (WebCore::Document::setFocusedElement):
+
 2018-02-12  John Wilander  <wilan...@apple.com>
 
         Resource Load Statistics: Classify resources as prevalent based on redirects to other prevalent resources

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.cpp (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.cpp	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.cpp	2018-02-20 07:45:39 UTC (rev 228745)
@@ -1017,6 +1017,14 @@
     postNotification(getOrCreate(node), &document(), AXMenuListItemSelected);
 }
     
+void AXObjectCache::deferFocusedUIElementChangeIfNeeded(Node* oldNode, Node* newNode)
+{
+    if (nodeAndRendererAreValid(newNode) && rendererNeedsDeferredUpdate(*newNode->renderer()))
+        m_deferredFocusedNodeChange.append({ oldNode, newNode });
+    else
+        handleFocusedUIElementChanged(oldNode, newNode);
+}
+    
 void AXObjectCache::handleFocusedUIElementChanged(Node* oldNode, Node* newNode)
 {
     handleMenuItemSelected(newNode);
@@ -2777,25 +2785,33 @@
     return result;
 }
 
-template<typename T, typename U>
-static void filterMapForRemoval(const HashMap<T, U>& list, const Document& document, HashSet<Node*>& nodesToRemove)
+static void conditionallyAddNodeToFilterList(Node* node, const Document& document, HashSet<Node*>& nodesToRemove)
 {
+    if (node && (!node->isConnected() || &node->document() == &document))
+        nodesToRemove.add(node);
+}
+    
+template<typename T>
+static void filterVectorPairForRemoval(const Vector<std::pair<T, T>>& list, const Document& document, HashSet<Node*>& nodesToRemove)
+{
     for (auto& entry : list) {
-        auto* node = entry.key;
-        if (node->isConnected() && &node->document() != &document)
-            continue;
-        nodesToRemove.add(node);
+        conditionallyAddNodeToFilterList(entry.first, document, nodesToRemove);
+        conditionallyAddNodeToFilterList(entry.second, document, nodesToRemove);
     }
 }
+    
+template<typename T, typename U>
+static void filterMapForRemoval(const HashMap<T, U>& list, const Document& document, HashSet<Node*>& nodesToRemove)
+{
+    for (auto& entry : list)
+        conditionallyAddNodeToFilterList(entry.key, document, nodesToRemove);
+}
 
 template<typename T>
 static void filterListForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove)
 {
-    for (auto* node : list) {
-        if (node->isConnected() && &node->document() != &document)
-            continue;
-        nodesToRemove.add(node);
-    }
+    for (auto* node : list)
+        conditionallyAddNodeToFilterList(node, document, nodesToRemove);
 }
 
 void AXObjectCache::prepareForDocumentDestruction(const Document& document)
@@ -2808,6 +2824,7 @@
     filterListForRemoval(m_deferredSelectedChildredChangedList, document, nodesToRemove);
     filterMapForRemoval(m_deferredTextFormControlValue, document, nodesToRemove);
     filterMapForRemoval(m_deferredAttributeChange, document, nodesToRemove);
+    filterVectorPairForRemoval(m_deferredFocusedNodeChange, document, nodesToRemove);
 
     for (auto* node : nodesToRemove)
         remove(*node);
@@ -2851,6 +2868,10 @@
     for (auto& deferredAttributeChangeContext : m_deferredAttributeChange)
         handleAttributeChange(deferredAttributeChangeContext.value, deferredAttributeChangeContext.key);
     m_deferredAttributeChange.clear();
+    
+    for (auto& deferredFocusedChangeContext : m_deferredFocusedNodeChange)
+        handleFocusedUIElementChanged(deferredFocusedChangeContext.first, deferredFocusedChangeContext.second);
+    m_deferredFocusedNodeChange.clear();
 }
     
 void AXObjectCache::deferRecomputeIsIgnoredIfNeeded(Element* element)

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.h (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.h	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.h	2018-02-20 07:45:39 UTC (rev 228745)
@@ -168,21 +168,13 @@
     void childrenChanged(RenderObject*, RenderObject* newChild = nullptr);
     void childrenChanged(AccessibilityObject*);
     void checkedStateChanged(Node*);
-    void selectedChildrenChanged(Node*);
-    void selectedChildrenChanged(RenderObject*);
-    // Called by a node when text or a text equivalent (e.g. alt) attribute is changed.
-    void textChanged(Node*);
     // Called when a node has just been attached, so we can make sure we have the right subclass of AccessibilityObject.
     void updateCacheAfterNodeIsAttached(Node*);
 
-    void handleActiveDescendantChanged(Node*);
-    void handleAriaRoleChanged(Node*);
-    void handleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
+    void deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
     void handleScrolledToAnchor(const Node* anchorNode);
-    void handleAriaExpandedChange(Node*);
     void handleScrollbarUpdate(ScrollView*);
     
-    void handleModalChange(Node*);
     Node* modalNode();
 
     void deferAttributeChangeIfNeeded(const QualifiedName&, Element*);
@@ -411,11 +403,20 @@
     void handleMenuItemSelected(Node*);
     void handleAttributeChange(const QualifiedName&, Element*);
     bool shouldProcessAttributeChange(const QualifiedName&, Element*);
-    
+    void selectedChildrenChanged(Node*);
+    void selectedChildrenChanged(RenderObject*);
+    // Called by a node when text or a text equivalent (e.g. alt) attribute is changed.
+    void textChanged(Node*);
+    void handleActiveDescendantChanged(Node*);
+    void handleAriaRoleChanged(Node*);
+    void handleAriaExpandedChange(Node*);
+    void handleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
+
     // aria-modal related
     void findModalNodes();
     void updateCurrentModalNode();
     bool isNodeVisible(Node*) const;
+    void handleModalChange(Node*);
 
     Document& m_document;
     HashMap<AXID, RefPtr<AccessibilityObject>> m_objects;
@@ -449,6 +450,7 @@
     ListHashSet<Element*> m_deferredSelectedChildredChangedList;
     HashMap<Element*, String> m_deferredTextFormControlValue;
     HashMap<Element*, QualifiedName> m_deferredAttributeChange;
+    Vector<std::pair<Node*, Node*>> m_deferredFocusedNodeChange;
     bool m_isSynchronizingSelection { false };
     bool m_performingDeferredCacheUpdate { false };
 };

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp (228744 => 228745)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp	2018-02-20 07:45:31 UTC (rev 228744)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp	2018-02-20 07:45:39 UTC (rev 228745)
@@ -3972,7 +3972,7 @@
     if (!focusChangeBlocked && m_focusedElement) {
         // Create the AXObject cache in a focus change because GTK relies on it.
         if (AXObjectCache* cache = axObjectCache())
-            cache->handleFocusedUIElementChanged(oldFocusedElement.get(), newFocusedElement.get());
+            cache->deferFocusedUIElementChangeIfNeeded(oldFocusedElement.get(), newFocusedElement.get());
     }
 
     if (!focusChangeBlocked && page())
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to