Changeset 228390 in webkit

Timestamp:

Feb 12, 2018 12:40:23 PM (6 years ago)

Author:

Matt Lewis

Message:

Unreviewed, rolling out r228376.

This caused accessibility/mac/selection-notification-focus-
change.html to become flaky on macOS.

Reverted changeset:

"AX: defer focusedUIElement notifications"
​https://bugs.webkit.org/show_bug.cgi?id=182643
​https://trac.webkit.org/changeset/228376

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html (modified) (1 diff)
• LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AXObjectCache.cpp (modified) (5 diffs)
• Source/WebCore/accessibility/AXObjectCache.h (modified) (3 diffs)
• Source/WebCore/dom/Document.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-02-12  Matt Lewis  <jlewis3@apple.com>
+
+        Unreviewed, rolling out r228376.
+
+        This caused accessibility/mac/selection-notification-focus-
+        change.html to become flaky on macOS.
+
+        Reverted changeset:
+
+        "AX: defer focusedUIElement notifications"
+        https://bugs.webkit.org/show_bug.cgi?id=182643
+        https://trac.webkit.org/changeset/228376
+
 2018-02-12  Per Arne Vollan  <pvollan@apple.com>
 

trunk/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html

@@ -47 +47 @@
         document.getElementById("item1").focus();
 
-        setTimeout(function() {
-            // Trigger notification through aria-selected.
-            document.getElementById("item2").setAttribute("aria-selected", "true");
+        // Trigger notification through aria-selected.
+        document.getElementById("item2").setAttribute("aria-selected", "true");
 
-            setTimeout(function() {
-                // Ensure we don't get a notification when aria-selected is false.
-                document.getElementById("item2").setAttribute("aria-selected", "false");
+        // Ensure we don't get a notification when aria-selected is false.
+        document.getElementById("item2").setAttribute("aria-selected", "false");
 
-                setTimeout(function() {
-                    // Trigger another notification through focus to ensure we don't
-                    document.getElementById("item3").focus();
-                }, 1);
-            }, 1);
-        }, 1);
+        // Trigger another notification through focus to ensure we don't
+        document.getElementById("item3").focus();
     }
 

trunk/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt

@@ -17 +17 @@
 
 eventSender.keyDown(tabCharacter)
+Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true
-Received AXFocusChanged
 Received AXSelectedTextChanged
 PASS userInfo["AXTextSelectionChangedFocus"] is true

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-12  Matt Lewis  <jlewis3@apple.com>
+
+        Unreviewed, rolling out r228376.
+
+        This caused accessibility/mac/selection-notification-focus-
+        change.html to become flaky on macOS.
+
+        Reverted changeset:
+
+        "AX: defer focusedUIElement notifications"
+        https://bugs.webkit.org/show_bug.cgi?id=182643
+        https://trac.webkit.org/changeset/228376
+
 2018-02-12  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -1016 +1016 @@
     
     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);
 }
     
@@ -2786 +2778 @@
 }
 
-static void conditionallyAddNodeToFilterList(Node* node, const Document& document, HashSet<Node*>& nodesToRemove)
-{
-    if (!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) {
-        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);
+    for (auto& entry : list) {
+        auto* node = entry.key;
+        if (node->isConnected() && &node->document() != &document)
+            continue;
+        nodesToRemove.add(node);
+    }
 }
 
@@ -2811 +2792 @@
 static void filterListForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove)
 {
-    for (auto* node : list)
-        conditionallyAddNodeToFilterList(node, document, nodesToRemove);
+    for (auto* node : list) {
+        if (node->isConnected() && &node->document() != &document)
+            continue;
+        nodesToRemove.add(node);
+    }
 }
 
@@ -2825 +2809 @@
     filterMapForRemoval(m_deferredTextFormControlValue, document, nodesToRemove);
     filterMapForRemoval(m_deferredAttributeChange, document, nodesToRemove);
-    filterVectorPairForRemoval(m_deferredFocusedNodeChange, document, nodesToRemove);
 
     for (auto* node : nodesToRemove)
@@ -2869 +2852 @@
         handleAttributeChange(deferredAttributeChangeContext.value, deferredAttributeChangeContext.key);
     m_deferredAttributeChange.clear();
-    
-    for (auto& deferredFocusedChangeContext : m_deferredFocusedNodeChange)
-        handleFocusedUIElementChanged(deferredFocusedChangeContext.first, deferredFocusedChangeContext.second);
-    m_deferredFocusedNodeChange.clear();
 }
     

trunk/Source/WebCore/accessibility/AXObjectCache.h

@@ -169 +169 @@
     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 deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
+    void handleActiveDescendantChanged(Node*);
+    void handleAriaRoleChanged(Node*);
+    void handleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
     void handleScrolledToAnchor(const Node* anchorNode);
+    void handleAriaExpandedChange(Node*);
     void handleScrollbarUpdate(ScrollView*);
     
+    void handleModalChange(Node*);
     Node* modalNode();
 
@@ -404 +412 @@
     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;
@@ -451 +450 @@
     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 };

trunk/Source/WebCore/dom/Document.cpp

@@ -3969 +3969 @@
         // Create the AXObject cache in a focus change because GTK relies on it.
         if (AXObjectCache* cache = axObjectCache())
-            cache->deferFocusedUIElementChangeIfNeeded(oldFocusedElement.get(), newFocusedElement.get());
+            cache->handleFocusedUIElementChanged(oldFocusedElement.get(), newFocusedElement.get());
     }