[AX] WebProcess from WebKitGtk+ 2.10.0 compiled in Debug mode hits ASSERT on textUnde...
authormario@webkit.org <mario@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Nov 2015 14:46:19 +0000 (14:46 +0000)
committermario@webkit.org <mario@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Nov 2015 14:46:19 +0000 (14:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150670

Source/WebCore:

Patch by Mario Sanchez Prada <mario@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 <jdiggs@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.

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

LayoutTests/ChangeLog
LayoutTests/accessibility/list-item-with-pseudo-element-crash-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/list-item-with-pseudo-element-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.cpp

index 9dcd501061111963025fd2cd9ad5d1b793b5badc..172b15dbc5505044173da3b2e6a7d06d59dc2b2a 100644 (file)
@@ -1,3 +1,13 @@
+2015-11-04  Joanmarie Diggs  <jdiggs@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  <calvaris@igalia.com>
 
         [Streams API] Shield streams against user replacing the Promise constructor
diff --git a/LayoutTests/accessibility/list-item-with-pseudo-element-crash-expected.txt b/LayoutTests/accessibility/list-item-with-pseudo-element-crash-expected.txt
new file mode 100644 (file)
index 0000000..4adff06
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/accessibility/list-item-with-pseudo-element-crash.html b/LayoutTests/accessibility/list-item-with-pseudo-element-crash.html
new file mode 100644 (file)
index 0000000..233b4f1
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<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="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 0c47e0baac491af5d0e8bb1c6fa99f93ca7754c4..94dfcef1784c216195060ee72e234257a6662604 100644 (file)
@@ -1,3 +1,22 @@
+2015-11-04  Mario Sanchez Prada  <mario@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  <calvaris@igalia.com>
 
         [Streams API] Shield streams against user replacing the Promise constructor
index 0719c1d61c3d1ffdbfb06e6e4b5c0c8bffc4c846..8abfbd516545803d7e8d662d98b926ba7e9549f8 100644 (file)
@@ -1687,12 +1687,6 @@ String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMo
     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)
index d5d3b644704f41809e38c3026285da93646761e2..da8b2bffefdf60d849960225c3033c89158af321 100644 (file)
@@ -686,6 +686,13 @@ String AccessibilityRenderObject::textUnderElement(AccessibilityTextUnderElement
                 // 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());
             }
         }