Title: [139444] trunk
- Revision
- 139444
- Author
- [email protected]
- Date
- 2013-01-11 07:35:06 -0800 (Fri, 11 Jan 2013)
Log Message
Objects can be re-added to the AXObjectCache during removal
https://bugs.webkit.org/show_bug.cgi?id=104171
Source/WebCore:
The problem occurs when a label's corresponding element is a sibling
that precedes it in the render tree, and the corresponding element is
removed. The corresponding element's AX render object is removed, but
then recreated when accessibilityIsIgnored() invokes correspondingControl()
on the label. The corresponding renderer then has an AX render object
that survives beyond the deleted renderer, leading to invalid memory
accesses.
The solution is to rearrange the calls to delete the renderer's AX
render object only when we are sure it will no longer be required.
Reviewed by Simon Fraser.
Test: accessibility/corresponding-control-deleted-crash.html
* rendering/RenderObject.cpp:
(WebCore::RenderObject::willBeDestroyed): Move the call to remove the
renderer from the AXCache to after the renderer is removed from the
render tree. This means that the AXObject still exists during renderer
removal, as we require.
LayoutTests:
Reviewed by Simon Fraser.
New test which asserts with !m_hasAXObject in RenderObject::~RenderObject without the patch. Requires Shadow DOM enabled.
* accessibility/corresponding-control-deleted-crash-expected.txt: Added.
* accessibility/corresponding-control-deleted-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (139443 => 139444)
--- trunk/LayoutTests/ChangeLog 2013-01-11 15:33:57 UTC (rev 139443)
+++ trunk/LayoutTests/ChangeLog 2013-01-11 15:35:06 UTC (rev 139444)
@@ -1,3 +1,15 @@
+2013-01-11 Stephen Chenney <[email protected]>
+
+ Objects can be re-added to the AXObjectCache during removal
+ https://bugs.webkit.org/show_bug.cgi?id=104171
+
+ Reviewed by Simon Fraser.
+
+ New test which asserts with !m_hasAXObject in RenderObject::~RenderObject without the patch. Requires Shadow DOM enabled.
+
+ * accessibility/corresponding-control-deleted-crash-expected.txt: Added.
+ * accessibility/corresponding-control-deleted-crash.html: Added.
+
2013-01-11 Sudarsana Nagineni <[email protected]>
Unreviewed EFL gardening.
Added: trunk/LayoutTests/accessibility/corresponding-control-deleted-crash-expected.txt (0 => 139444)
--- trunk/LayoutTests/accessibility/corresponding-control-deleted-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/corresponding-control-deleted-crash-expected.txt 2013-01-11 15:35:06 UTC (rev 139444)
@@ -0,0 +1,9 @@
+Make sure that a debug assert is not triggered when a call to RenderBlock::deleteLineBoxTree calls AccessibilityRenderObject::accessibilityIsIgnored which may require the AXObject for a node that is being deleted.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/corresponding-control-deleted-crash.html (0 => 139444)
--- trunk/LayoutTests/accessibility/corresponding-control-deleted-crash.html (rev 0)
+++ trunk/LayoutTests/accessibility/corresponding-control-deleted-crash.html 2013-01-11 15:35:06 UTC (rev 139444)
@@ -0,0 +1,42 @@
+
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+ description("Make sure that a debug assert is not triggered when a call to RenderBlock::deleteLineBoxTree calls AccessibilityRenderObject::accessibilityIsIgnored which may require the AXObject for a node that is being deleted.");
+
+ var label = document.createElement('label');
+ label.style.position = 'fixed';
+ document.body.appendChild(label);
+
+ var progress = document.createElement('progress');
+ progress.style.display = 'block';
+ label.appendChild(progress);
+
+ var kbd = document.createElement('kbd');
+ label.appendChild(kbd);
+
+ var labelShadow = label.webkitCreateShadowRoot();
+
+ var select = document.createElement('select');
+ select.setAttribute('multiple', 'multiple');
+ labelShadow.appendChild(select);
+
+ var shadow = document.createElement('shadow');
+ labelShadow.appendChild(shadow);
+
+ select.focus();
+
+ document.body.removeChild(label);
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (139443 => 139444)
--- trunk/Source/WebCore/ChangeLog 2013-01-11 15:33:57 UTC (rev 139443)
+++ trunk/Source/WebCore/ChangeLog 2013-01-11 15:35:06 UTC (rev 139444)
@@ -1,3 +1,28 @@
+2013-01-11 Stephen Chenney <[email protected]>
+ Objects can be re-added to the AXObjectCache during removal
+ https://bugs.webkit.org/show_bug.cgi?id=104171
+
+ The problem occurs when a label's corresponding element is a sibling
+ that precedes it in the render tree, and the corresponding element is
+ removed. The corresponding element's AX render object is removed, but
+ then recreated when accessibilityIsIgnored() invokes correspondingControl()
+ on the label. The corresponding renderer then has an AX render object
+ that survives beyond the deleted renderer, leading to invalid memory
+ accesses.
+
+ The solution is to rearrange the calls to delete the renderer's AX
+ render object only when we are sure it will no longer be required.
+
+ Reviewed by Simon Fraser.
+
+ Test: accessibility/corresponding-control-deleted-crash.html
+
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::willBeDestroyed): Move the call to remove the
+ renderer from the AXCache to after the renderer is removed from the
+ render tree. This means that the AXObject still exists during renderer
+ removal, as we require.
+
2013-01-11 Allan Sandfeld Jensen <[email protected]>
[Qt][WK1] Web Audio support
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (139443 => 139444)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2013-01-11 15:33:57 UTC (rev 139443)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2013-01-11 15:35:06 UTC (rev 139444)
@@ -2370,14 +2370,20 @@
if (frame() && frame()->eventHandler()->autoscrollRenderer() == this)
frame()->eventHandler()->stopAutoscrollTimer(true);
- if (AXObjectCache::accessibilityEnabled()) {
- document()->axObjectCache()->childrenChanged(this->parent());
- document()->axObjectCache()->remove(this);
- }
animation()->cancelAnimations(this);
+ // For accessibility management, notify the parent of the imminent change to its child set.
+ // We do it now, before remove(), while the parent pointer is still available.
+ if (AXObjectCache::accessibilityEnabled())
+ document()->axObjectCache()->childrenChanged(this->parent());
+
remove();
+ // The remove() call above may invoke axObjectCache()->childrenChanged() on the parent, which may require the AX render
+ // object for this renderer. So we remove the AX render object now, after the renderer is removed.
+ if (AXObjectCache::accessibilityEnabled())
+ document()->axObjectCache()->remove(this);
+
// Continuation and first-letter can generate several renderers associated with a single node.
// We only want to clear the node's renderer if we are the associated renderer.
if (node() && node()->renderer() == this)
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
