Source/WebCore: Clicking on links while accessibility is enabled sometimes crashes
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jul 2014 17:50:31 +0000 (17:50 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jul 2014 17:50:31 +0000 (17:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135074

Reviewed by Chris Fleizach.

When an accessibility request comes in from the system, we call updateBackingStore() on the
relevant AccessibilityObject, which triggers a relayout of the entire document. This relayout
might delete that accessibility node and its parent, which would cause the node to be deleted.
After the stack unwinds, we then call a member function on the node without checking for this
condition.

Test: accessibility/parent-delete.html

* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::updateBackingStore): Retain the node for the duration of the
function.

LayoutTests: Clicking on links while accessibility is enabled does not render as expected
https://bugs.webkit.org/show_bug.cgi?id=135074

Reviewed by Chris Fleizach.

Delete a node and its parent, then call allAttributes() on the accessibility representation of
the deleted child and make sure there is no crash.

* accessibility/parent-delete-expected.txt: Added
* accessibility/parent-delete.html: Added

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

LayoutTests/ChangeLog
LayoutTests/accessibility/parent-delete-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/parent-delete.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityObject.cpp

index f0394dcfb30fc669061f621e33feeb09b50bed7e..da788223f01915f8ba1a01170d2761968ab3d8ea 100644 (file)
@@ -1,3 +1,16 @@
+2014-07-21  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Clicking on links while accessibility is enabled does not render as expected
+        https://bugs.webkit.org/show_bug.cgi?id=135074
+
+        Reviewed by Chris Fleizach.
+
+        Delete a node and its parent, then call allAttributes() on the accessibility representation of
+        the deleted child and make sure there is no crash.
+
+        * accessibility/parent-delete-expected.txt: Added
+        * accessibility/parent-delete.html: Added
+
 2014-07-22  Alexey Proskuryakov  <ap@apple.com>
 
         media/track/track-in-band-subtitles-too-large.html and
diff --git a/LayoutTests/accessibility/parent-delete-expected.txt b/LayoutTests/accessibility/parent-delete-expected.txt
new file mode 100644 (file)
index 0000000..79e7ec9
--- /dev/null
@@ -0,0 +1,31 @@
+This test passes if there is no crash.
+AXRole: AXWebArea
+AXRoleDescription: HTML content
+AXChildren: <array of size 1>
+AXHelp: 
+AXParent: <AXWebArea>
+AXSize: NSSize: {800, 600}
+AXTitle: 
+AXDescription: 
+AXValue: 
+AXFocused: 0
+AXEnabled: 1
+AXWindow: <AXWebArea>
+AXSelectedTextMarkerRange: (null)
+AXStartTextMarker: <AXWebArea>
+AXEndTextMarker: <AXWebArea>
+AXVisited: 0
+AXLinkedUIElements: (null)
+AXSelected: 0
+AXBlockQuoteLevel: 0
+AXTopLevelUIElement: <AXWebArea>
+AXLanguage: 
+AXDOMIdentifier: 
+AXDOMClassList: <array of size 0>
+AXLinkUIElements: <array of size 0>
+AXLoaded: 1
+AXLayoutCount: 2
+AXLoadingProgress: 1
+AXURL: LayoutTests/accessibility/parent-delete.html
+AXElementBusy: 0
+
diff --git a/LayoutTests/accessibility/parent-delete.html b/LayoutTests/accessibility/parent-delete.html
new file mode 100644 (file)
index 0000000..4d7c5e2
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function runTest() {
+    var accessibilityElement;
+    {
+        var outer = document.getElementById("outer");
+        var inner = document.getElementById("inner");
+        var editable = document.getElementById("editable");
+        var result = document.getElementById("result");
+        editable.focus();
+        if (window.accessibilityController) {
+            var accessibilityElement = accessibilityController.focusedElement;
+        }
+        inner.removeChild(editable);
+        outer.removeChild(inner);
+    }
+    if (window.accessibilityController) {
+        result.innerText = accessibilityElement.allAttributes();
+    }
+}
+</script>
+</head>
+<body onload="runTest()">
+This test passes if there is no crash.
+<div id="outer" style="display: none;">
+    <div id="inner" style="display: none;">
+        <div id="editable" contenteditable="true" style="display: none;">
+            This is some throwaway text
+        </div>
+    </div>
+</div>
+<div id="result"></div>
+</body>
+</html>
index 5f04d333fa23b6083e060c49bb4f25a61531c9d9..aa0b9ab3d26dc4f51298064b75c851894b526624 100644 (file)
@@ -1,3 +1,22 @@
+2014-07-21  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Clicking on links while accessibility is enabled sometimes crashes
+        https://bugs.webkit.org/show_bug.cgi?id=135074
+
+        Reviewed by Chris Fleizach.
+
+        When an accessibility request comes in from the system, we call updateBackingStore() on the
+        relevant AccessibilityObject, which triggers a relayout of the entire document. This relayout
+        might delete that accessibility node and its parent, which would cause the node to be deleted.
+        After the stack unwinds, we then call a member function on the node without checking for this
+        condition.
+
+        Test: accessibility/parent-delete.html
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::updateBackingStore): Retain the node for the duration of the
+        function.
+
 2014-07-22  Jeremy Jones  <jeremyj@apple.com>
 
         Don't create new UIWindow for video fullscreen.
index 9fff8f140d93f53dd9779c32cbc7282bbe72b4c5..9566b2661992301a12f2df722255c61650386a49 100644 (file)
@@ -1429,6 +1429,8 @@ unsigned AccessibilityObject::doAXLineForIndex(unsigned index)
 void AccessibilityObject::updateBackingStore()
 {
     // Updating the layout may delete this object.
+    RefPtr<AccessibilityObject> protector(this);
+
     if (Document* document = this->document()) {
         if (!document->view()->isInLayout())
             document->updateLayoutIgnorePendingStylesheets();