Changeset 216096 in webkit

Timestamp:

May 2, 2017 2:29:13 PM (7 years ago)

Author:

Alan Bujtas

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/crash-while-adding-text-child-with-transform-expected.txt (added)
• LayoutTests/accessibility/crash-while-adding-text-child-with-transform.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AXObjectCache.cpp (modified) (2 diffs)
• Source/WebCore/accessibility/AXObjectCache.h (modified) (4 diffs)
• Source/WebCore/page/FrameView.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderText.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -712 +712 @@
     remove(axID);
     m_renderObjectMapping.remove(renderer);
-    if (is<RenderBlock>(*renderer))
-        m_deferredIsIgnoredChangeList.remove(downcast<RenderBlock>(renderer));
+    m_deferredCacheUpdateList.remove(renderer);
 }
 
@@ -2701 +2700 @@
 }
     
-void AXObjectCache::performDeferredIsIgnoredChange()
-{
-    for (auto* renderer : m_deferredIsIgnoredChangeList)
-        recomputeIsIgnored(renderer);
-    m_deferredIsIgnoredChangeList.clear();
+void AXObjectCache::performDeferredCacheUpdate()
+{
+    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);
 }
 

trunk/Source/WebCore/accessibility/AXObjectCache.h

@@ -46 +46 @@
 class RenderBlock;
 class RenderObject;
+class RenderText;
 class ScrollView;
 class VisiblePosition;
@@ -328 +329 @@
 #endif
     void recomputeDeferredIsIgnored(RenderBlock& renderer);
-    void performDeferredIsIgnoredChange();
+    void deferTextChanged(RenderText& renderer);
+    void performDeferredCacheUpdate();
 
 protected:
@@ -430 +432 @@
     AXTextStateChangeIntent m_textSelectionIntent;
     bool m_isSynchronizingSelection { false };
-    ListHashSet<RenderBlock*> m_deferredIsIgnoredChangeList;
+    ListHashSet<RenderObject*> m_deferredCacheUpdateList;
 };
 
@@ -493 +495 @@
 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&) { }

trunk/Source/WebCore/page/FrameView.cpp

@@ -3554 +3554 @@
 
     if (AXObjectCache* cache = frame().document()->existingAXObjectCache())
-        cache->performDeferredIsIgnoredChange();
+        cache->performDeferredCacheUpdate();
 }
 

trunk/Source/WebCore/rendering/RenderText.cpp

@@ -1282 +1282 @@
     
     if (AXObjectCache* cache = document().existingAXObjectCache())
-        cache->textChanged(this);
+        cache->deferTextChanged(*this);
 }