Defer AX cache update when text content changes until after layout is finished.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 May 2017 21:29:13 +0000 (21:29 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 May 2017 21:29:13 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171429
<rdar://problem/31885984>

Reviewed by Simon Fraser.

Source/WebCore:

When the content of the RenderText changes (even as the result of a text-transform change)
instead of updating the AX cache eagerly (and trigger layout on a half-backed render tree)
we should just defer it until after the subsequent layout is done.

Test: accessibility/crash-while-adding-text-child-with-transform.html

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::performDeferredCacheUpdate):
(WebCore::AXObjectCache::recomputeDeferredIsIgnored):
(WebCore::AXObjectCache::deferTextChanged):
(WebCore::AXObjectCache::performDeferredIsIgnoredChange): Deleted.
* accessibility/AXObjectCache.h:
(WebCore::AXObjectCache::deferTextChanged):
(WebCore::AXObjectCache::performDeferredCacheUpdate):
(WebCore::AXObjectCache::performDeferredIsIgnoredChange): Deleted.
* page/FrameView.cpp:
(WebCore::FrameView::performPostLayoutTasks):
* rendering/RenderText.cpp:
(WebCore::RenderText::setText):

LayoutTests:

* accessibility/crash-while-adding-text-child-with-transform-expected.txt: Added.
* accessibility/crash-while-adding-text-child-with-transform.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/accessibility/crash-while-adding-text-child-with-transform-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/crash-while-adding-text-child-with-transform.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/rendering/RenderText.cpp

index 7798fa2..dca8647 100644 (file)
@@ -1,3 +1,14 @@
+2017-05-02  Zalan Bujtas  <zalan@apple.com>
+
+        Defer AX cache update when text content changes until after layout is finished.
+        https://bugs.webkit.org/show_bug.cgi?id=171429
+        <rdar://problem/31885984>
+
+        Reviewed by Simon Fraser.
+
+        * accessibility/crash-while-adding-text-child-with-transform-expected.txt: Added.
+        * accessibility/crash-while-adding-text-child-with-transform.html: Added.
+
 2017-05-02  David Kilzer  <ddkilzer@apple.com>
 
         check-webkit-style should keep JavaScript test functions in sync
diff --git a/LayoutTests/accessibility/crash-while-adding-text-child-with-transform-expected.txt b/LayoutTests/accessibility/crash-while-adding-text-child-with-transform-expected.txt
new file mode 100644 (file)
index 0000000..29afa38
--- /dev/null
@@ -0,0 +1,2 @@
+pass if no crash or assert.
+
diff --git a/LayoutTests/accessibility/crash-while-adding-text-child-with-transform.html b/LayoutTests/accessibility/crash-while-adding-text-child-with-transform.html
new file mode 100644 (file)
index 0000000..69df3cd
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that AX can manage text content changes while inside tree mutation.</title>
+<style>
+#foobar {
+    text-transform: lowercase;
+}
+
+#foobar::first-letter {
+       margin: 1px;
+}
+</style>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    if (window.accessibilityController)
+        accessibilityController.accessibleElementById("foobar");
+</script>
+</head>
+<body>
+<div id=foobar>Pass if no crash or assert.</div><span aria-labeledby=foobar id=newParent></span><script>
+    newParent.appendChild(foobar);
+</script>
+</body>
+</html>
index d8fdc25..f2bfd29 100644 (file)
@@ -1,3 +1,32 @@
+2017-05-02  Zalan Bujtas  <zalan@apple.com>
+
+        Defer AX cache update when text content changes until after layout is finished.
+        https://bugs.webkit.org/show_bug.cgi?id=171429
+        <rdar://problem/31885984>
+
+        Reviewed by Simon Fraser.
+
+        When the content of the RenderText changes (even as the result of a text-transform change)
+        instead of updating the AX cache eagerly (and trigger layout on a half-backed render tree)
+        we should just defer it until after the subsequent layout is done. 
+
+        Test: accessibility/crash-while-adding-text-child-with-transform.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::remove):
+        (WebCore::AXObjectCache::performDeferredCacheUpdate):
+        (WebCore::AXObjectCache::recomputeDeferredIsIgnored):
+        (WebCore::AXObjectCache::deferTextChanged):
+        (WebCore::AXObjectCache::performDeferredIsIgnoredChange): Deleted.
+        * accessibility/AXObjectCache.h:
+        (WebCore::AXObjectCache::deferTextChanged):
+        (WebCore::AXObjectCache::performDeferredCacheUpdate):
+        (WebCore::AXObjectCache::performDeferredIsIgnoredChange): Deleted.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::performPostLayoutTasks):
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::setText):
+
 2017-05-02  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Remove an extraneous call to dispatch_group_async in WebItemProviderPasteboard.mm
index f35ef90..7fdb9cf 100644 (file)
@@ -711,8 +711,7 @@ void AXObjectCache::remove(RenderObject* renderer)
     AXID axID = m_renderObjectMapping.get(renderer);
     remove(axID);
     m_renderObjectMapping.remove(renderer);
-    if (is<RenderBlock>(*renderer))
-        m_deferredIsIgnoredChangeList.remove(downcast<RenderBlock>(renderer));
+    m_deferredCacheUpdateList.remove(renderer);
 }
 
 void AXObjectCache::remove(Node* node)
@@ -2700,16 +2699,27 @@ bool AXObjectCache::nodeIsTextControl(const Node* node)
     return axObject && axObject->isTextControl();
 }
     
-void AXObjectCache::performDeferredIsIgnoredChange()
+void AXObjectCache::performDeferredCacheUpdate()
 {
-    for (auto* renderer : m_deferredIsIgnoredChangeList)
-        recomputeIsIgnored(renderer);
-    m_deferredIsIgnoredChangeList.clear();
+    for (auto* renderer : m_deferredCacheUpdateList) {
+        if (is<RenderBlock>(*renderer))
+            recomputeIsIgnored(renderer);
+        else if (is<RenderText>(*renderer))
+            textChanged(renderer);
+        else
+            ASSERT_NOT_REACHED();
+    }
+    m_deferredCacheUpdateList.clear();
 }
 
 void AXObjectCache::recomputeDeferredIsIgnored(RenderBlock& renderer)
 {
-    m_deferredIsIgnoredChangeList.add(&renderer);
+    m_deferredCacheUpdateList.add(&renderer);
+}
+
+void AXObjectCache::deferTextChanged(RenderText& renderer)
+{
+    m_deferredCacheUpdateList.add(&renderer);
 }
 
 bool isNodeAriaVisible(Node* node)
index e7c332d..19ed0f5 100644 (file)
@@ -45,6 +45,7 @@ class Node;
 class Page;
 class RenderBlock;
 class RenderObject;
+class RenderText;
 class ScrollView;
 class VisiblePosition;
 class Widget;
@@ -327,7 +328,8 @@ public:
     static void setShouldRepostNotificationsForTests(bool value);
 #endif
     void recomputeDeferredIsIgnored(RenderBlock& renderer);
-    void performDeferredIsIgnoredChange();
+    void deferTextChanged(RenderText& renderer);
+    void performDeferredCacheUpdate();
 
 protected:
     void postPlatformNotification(AccessibilityObject*, AXNotification);
@@ -429,7 +431,7 @@ private:
 
     AXTextStateChangeIntent m_textSelectionIntent;
     bool m_isSynchronizingSelection { false };
-    ListHashSet<RenderBlock*> m_deferredIsIgnoredChangeList;
+    ListHashSet<RenderObject*> m_deferredCacheUpdateList;
 };
 
 class AXAttributeCacheEnabler
@@ -492,7 +494,8 @@ inline void AXObjectCache::handleScrollbarUpdate(ScrollView*) { }
 inline void AXObjectCache::handleAttributeChanged(const QualifiedName&, Element*) { }
 inline void AXObjectCache::recomputeIsIgnored(RenderObject*) { }
 inline void AXObjectCache::recomputeDeferredIsIgnored(RenderBlock&) { }
-inline void AXObjectCache::performDeferredIsIgnoredChange() { }
+inline void AXObjectCache::deferTextChanged(RenderText&) { }
+inline void AXObjectCache::performDeferredCacheUpdate() { }
 inline void AXObjectCache::handleScrolledToAnchor(const Node*) { }
 inline void AXObjectCache::postTextStateChangeNotification(Node*, const AXTextStateChangeIntent&, const VisibleSelection&) { }
 inline void AXObjectCache::postTextStateChangeNotification(Node*, AXTextEditType, const String&, const VisiblePosition&) { }
index e0541dc..7f3fe56 100644 (file)
@@ -3553,7 +3553,7 @@ void FrameView::performPostLayoutTasks()
     updateScrollSnapState();
 
     if (AXObjectCache* cache = frame().document()->existingAXObjectCache())
-        cache->performDeferredIsIgnoredChange();
+        cache->performDeferredCacheUpdate();
 }
 
 IntSize FrameView::sizeForResizeEvent() const
index 2348b3c..72b1c02 100644 (file)
@@ -1281,7 +1281,7 @@ void RenderText::setText(const String& text, bool force)
         downcast<RenderBlockFlow>(*parent()).invalidateLineLayoutPath();
     
     if (AXObjectCache* cache = document().existingAXObjectCache())
-        cache->textChanged(this);
+        cache->deferTextChanged(*this);
 }
 
 String RenderText::textWithoutConvertingBackslashToYenSymbol() const