Title: [192022] trunk
Revision
192022
Author
ma...@webkit.org
Date
2015-11-04 06:46:19 -0800 (Wed, 04 Nov 2015)

Log Message

[AX] WebProcess from WebKitGtk+ 2.10.0 compiled in Debug mode hits ASSERT on textUnderElement
https://bugs.webkit.org/show_bug.cgi?id=150670

Source/WebCore:

Patch by Mario Sanchez Prada <ma...@webkit.org> on 2015-11-04
Reviewed by Chris Fleizach.

Move the ASSERTs stating that the render tree is stable before using the
TextIterator to their right place, in AccessibilityRenderObject, so that
we don't crash in debug builds in cases when this condition is irrelevant.

Test: accessibility/gtk/list-item-with-pseudo-element-crash.html

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::textUnderElement): Removed ASSERTs.
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::textUnderElement): Added ASSERTs, but
only before calling plainText and using the right document for the node.

LayoutTests:

Patch by Joanmarie Diggs <jdi...@igalia.com> on 2015-11-04
Reviewed by Chris Fleizach.

* accessibility/list-item-with-pseudo-element-crash-expected.txt: Added.
* accessibility/list-item-with-pseudo-element-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (192021 => 192022)


--- trunk/LayoutTests/ChangeLog	2015-11-04 11:15:51 UTC (rev 192021)
+++ trunk/LayoutTests/ChangeLog	2015-11-04 14:46:19 UTC (rev 192022)
@@ -1,3 +1,13 @@
+2015-11-04  Joanmarie Diggs  <jdi...@igalia.com>
+
+        [AX] WebProcess from WebKitGtk+ 2.10.0 compiled in Debug mode hits ASSERT on textUnderElement
+        https://bugs.webkit.org/show_bug.cgi?id=150670
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/list-item-with-pseudo-element-crash-expected.txt: Added.
+        * accessibility/list-item-with-pseudo-element-crash.html: Added.
+
 2015-11-04  Xabier Rodriguez Calvar  <calva...@igalia.com>
 
         [Streams API] Shield streams against user replacing the Promise constructor

Added: trunk/LayoutTests/accessibility/list-item-with-pseudo-element-crash-expected.txt (0 => 192022)


--- trunk/LayoutTests/accessibility/list-item-with-pseudo-element-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/list-item-with-pseudo-element-crash-expected.txt	2015-11-04 14:46:19 UTC (rev 192022)
@@ -0,0 +1,11 @@
+Foo
+BAR
+This verifies that list items with pseudo elements and styles do not trigger an assertion.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/accessibility/list-item-with-pseudo-element-crash.html (0 => 192022)


--- trunk/LayoutTests/accessibility/list-item-with-pseudo-element-crash.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/list-item-with-pseudo-element-crash.html	2015-11-04 14:46:19 UTC (rev 192022)
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+<style>
+.item:before {content:""; }
+</style>
+</head>
+<body>
+<ul id="list" style="display:none;">
+<li id="item1" class="item" style="float:left;">Foo</li>
+<li id="item2" class="item" style="text-transform:uppercase;">Bar</li>
+</ul>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("This verifies that list items with pseudo elements and styles do not trigger an assertion.");
+
+if (window.testRunner && window.accessibilityController) {
+    var list = document.getElementById("list");
+    list.style.display = "";
+}
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (192021 => 192022)


--- trunk/Source/WebCore/ChangeLog	2015-11-04 11:15:51 UTC (rev 192021)
+++ trunk/Source/WebCore/ChangeLog	2015-11-04 14:46:19 UTC (rev 192022)
@@ -1,3 +1,22 @@
+2015-11-04  Mario Sanchez Prada  <ma...@webkit.org>
+
+        [AX] WebProcess from WebKitGtk+ 2.10.0 compiled in Debug mode hits ASSERT on textUnderElement
+        https://bugs.webkit.org/show_bug.cgi?id=150670
+
+        Reviewed by Chris Fleizach.
+
+        Move the ASSERTs stating that the render tree is stable before using the
+        TextIterator to their right place, in AccessibilityRenderObject, so that
+        we don't crash in debug builds in cases when this condition is irrelevant.
+
+        Test: accessibility/gtk/list-item-with-pseudo-element-crash.html
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::textUnderElement): Removed ASSERTs.
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::textUnderElement): Added ASSERTs, but
+        only before calling plainText and using the right document for the node.
+
 2015-11-04  Xabier Rodriguez Calvar  <calva...@igalia.com>
 
         [Streams API] Shield streams against user replacing the Promise constructor

Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp (192021 => 192022)


--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp	2015-11-04 11:15:51 UTC (rev 192021)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp	2015-11-04 14:46:19 UTC (rev 192022)
@@ -1687,12 +1687,6 @@
     if (is<Text>(node))
         return downcast<Text>(*node).wholeText();
 
-    // The render tree should be stable before going ahead. Otherwise, further uses of the
-    // TextIterator will force a layout update, potentially altering the accessibility tree
-    // and leading to crashes in the loop that computes the result text from the children.
-    ASSERT(!document()->renderView()->layoutState());
-    ASSERT(!document()->childNeedsStyleRecalc());
-
     StringBuilder builder;
     for (AccessibilityObject* child = firstChild(); child; child = child->nextSibling()) {
         if (mode.ignoredChildNode && child->node() == mode.ignoredChildNode)

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (192021 => 192022)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2015-11-04 11:15:51 UTC (rev 192021)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2015-11-04 14:46:19 UTC (rev 192022)
@@ -686,6 +686,13 @@
                 // catch stale WebCoreAXObject (see <rdar://problem/3960196>)
                 if (frame->document() != nodeDocument)
                     return String();
+
+                // The render tree should be stable before going ahead. Otherwise, further uses of the
+                // TextIterator will force a layout update, potentially altering the accessibility tree
+                // and leading to crashes in the loop that computes the result text from the children.
+                ASSERT(!nodeDocument->renderView()->layoutState());
+                ASSERT(!nodeDocument->childNeedsStyleRecalc());
+
                 return plainText(textRange.get(), textIteratorBehaviorForTextRange());
             }
         }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to