Title: [171347] trunk
Revision
171347
Author
mmaxfi...@apple.com
Date
2014-07-22 10:50:31 -0700 (Tue, 22 Jul 2014)

Log Message

Source/WebCore: 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.

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (171346 => 171347)


--- trunk/LayoutTests/ChangeLog	2014-07-22 17:32:38 UTC (rev 171346)
+++ trunk/LayoutTests/ChangeLog	2014-07-22 17:50:31 UTC (rev 171347)
@@ -1,3 +1,16 @@
+2014-07-21  Myles C. Maxfield  <mmaxfi...@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  <a...@apple.com>
 
         media/track/track-in-band-subtitles-too-large.html and

Added: trunk/LayoutTests/accessibility/parent-delete-expected.txt (0 => 171347)


--- trunk/LayoutTests/accessibility/parent-delete-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/parent-delete-expected.txt	2014-07-22 17:50:31 UTC (rev 171347)
@@ -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
+

Added: trunk/LayoutTests/accessibility/parent-delete.html (0 => 171347)


--- trunk/LayoutTests/accessibility/parent-delete.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/parent-delete.html	2014-07-22 17:50:31 UTC (rev 171347)
@@ -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>

Modified: trunk/Source/WebCore/ChangeLog (171346 => 171347)


--- trunk/Source/WebCore/ChangeLog	2014-07-22 17:32:38 UTC (rev 171346)
+++ trunk/Source/WebCore/ChangeLog	2014-07-22 17:50:31 UTC (rev 171347)
@@ -1,3 +1,22 @@
+2014-07-21  Myles C. Maxfield  <mmaxfi...@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  <jere...@apple.com>
 
         Don't create new UIWindow for video fullscreen.

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (171346 => 171347)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2014-07-22 17:32:38 UTC (rev 171346)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2014-07-22 17:50:31 UTC (rev 171347)
@@ -1429,6 +1429,8 @@
 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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to