Bug 126073 - Crashes in AccessibilityRenderObject::computeAccessibilityIsIgnored()
Summary: Crashes in AccessibilityRenderObject::computeAccessibilityIsIgnored()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 126118
Blocks:
  Show dependency tree / graph
 
Reported: 2013-12-20 10:47 PST by Antti Koivisto
Modified: 2013-12-21 12:24 PST (History)
17 users (show)

See Also:

Attachments
patch (3.50 KB, patch)
2013-12-20 10:53 PST, Antti Koivisto 		rniwa: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Radar WebKit Bug Importer 2013-12-20 10:48:09 PST 
<rdar://problem/15709541>
Comment 2 Antti Koivisto 2013-12-20 10:53:49 PST 
Created attachment 219770 [details]
patch
Comment 3 Ryosuke Niwa 2013-12-20 10:56:14 PST 
Comment on attachment 219770 [details]
patch

rs=me
Comment 4 EFL EWS Bot 2013-12-20 10:58:30 PST 
Comment on attachment 219770 [details]
patch

Attachment 219770 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/44378325
Comment 5 Alexey Proskuryakov 2013-12-20 10:59:13 PST 
Comment on attachment 219770 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=219770&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1192
> +    // FIXME: Somehow the renderer is becoming null.

I'd say:

// FIXME (bug number): Some regression tests flakily crash without this check. It is not clear whether the renderer may legitimately be null here.
Comment 6 EFL EWS Bot 2013-12-20 11:02:36 PST 
Comment on attachment 219770 [details]
patch

Attachment 219770 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/49338291
Comment 7 Antti Koivisto 2013-12-20 11:02:46 PST 
https://trac.webkit.org/r160916
Comment 8 chris fleizach 2013-12-20 11:37:46 PST 
This is a bit risky to put in without having someone familiar with accessibility review it. I'm not sure what kind of negative effects it will have at all.

Was another bug opened to have someone look at this again?

Rather than ramming the other patch down WebKit and putting in temporary hacks, it might be better to work through the problems in tandem before committing
Comment 9 Antti Koivisto 2013-12-20 12:56:58 PST 
(In reply to comment #8)
> This is a bit risky to put in without having someone familiar with accessibility review it. I'm not sure what kind of negative effects it will have at all.
> 
> Was another bug opened to have someone look at this again?
> 
> Rather than ramming the other patch down WebKit and putting in temporary hacks, it might be better to work through the problems in tandem before committing

We are trying to understand the crash. The changes are temporary.
Comment 10 Antti Koivisto 2013-12-20 12:58:47 PST 
This is fallout from http://trac.webkit.org/changeset/160908
Comment 12 Antti Koivisto 2013-12-20 14:12:55 PST 
reopening
Comment 13 Antti Koivisto 2013-12-21 04:50:14 PST 
This looks like the root cause:

http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r160955%20(1283)/fast/events/nested-event-remove-node-crash-crash-log.txt

12  com.apple.WebCore             	0x00000001079381c8 WebCore::Style::resolveTree(WebCore::Document&, WebCore::Style::Change) + 520 (StyleResolveTree.cpp:880)
13  com.apple.WebCore             	0x0000000107c4ace6 WebCore::Document::recalcStyle(WebCore::Style::Change) + 438 (Document.cpp:1760)
14  com.apple.WebCore             	0x0000000107c474df WebCore::Document::updateStyleIfNeeded() + 431 (Document.cpp:1809)
15  com.apple.WebCore             	0x00000001080d16df WebCore::HTMLElement::supportsFocus() const + 111 (HTMLElement.cpp:667)
16  com.apple.WebCore             	0x00000001077169c6 WebCore::AccessibilityNodeObject::canSetFocusAttribute() const + 214 (AccessibilityNodeObject.cpp:1985)
17  com.apple.WebCore             	0x0000000107723839 WebCore::AccessibilityRenderObject::inheritsPresentationalRole() const + 41 (AccessibilityRenderObject.cpp:2625)
18  com.apple.WebCore             	0x0000000107722d95 WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const + 325 (AccessibilityRenderObject.cpp:1183)
19  com.apple.WebCore             	0x000000010771e84f WebCore::AccessibilityObject::accessibilityIsIgnored() const + 191 (AccessibilityObject.cpp:2133)
20  com.apple.WebCore             	0x00000001077080cd WebCore::AccessibilityObject::notifyIfIgnoredValueChanged() + 29 (AccessibilityObject.cpp:2033)
21  com.apple.WebCore             	0x0000000107708dc8 WebCore::AXObjectCache::recomputeIsIgnored(WebCore::RenderObject*) + 56 (AXObjectCache.cpp:864)
22  com.apple.WebCore             	0x0000000108be0359 WebCore::RenderBlock::deleteLines() + 73 (RenderBlock.cpp:932)
23  com.apple.WebCore             	0x0000000109462ea2 WebCore::RenderBlockFlow::deleteLines() + 450 (RenderBlockFlow.cpp:1827)
24  com.apple.WebCore             	0x0000000108be10cd WebCore::RenderBlock::removeChild(WebCore::RenderObject&) + 1709 (RenderBlock.cpp:1191)
25  com.apple.WebCore             	0x0000000108d91e06 WebCore::RenderObject::removeFromParent() + 70 (RenderObject.cpp:187)
26  com.apple.WebCore             	0x0000000108d9c2f3 WebCore::RenderObject::willBeDestroyed() + 163 (RenderObject.cpp:1865)
27  com.apple.WebCore             	0x0000000108e77647 WebCore::RenderText::willBeDestroyed() + 167 (RenderText.cpp:284)
28  com.apple.WebCore             	0x0000000108d9c8bd WebCore::RenderObject::destroy() + 29 (RenderObject.cpp:1985)
29  com.apple.WebCore             	0x0000000108d9c892 WebCore::RenderObject::destroyAndCleanupAnonymousWrappers() + 274 (RenderObject.cpp:1975)
30  com.apple.WebCore             	0x0000000107937012 WebCore::Style::detachTextRenderer(WebCore::Text&) + 50 (StyleResolveTree.cpp:414)
31  com.apple.WebCore             	0x0000000107938661 WebCore::Style::detachChildren(WebCore::ContainerNode&, WebCore::Style::DetachType) + 81 

We are entering style resolve from the middle of render tree destruction.
Comment 14 chris fleizach 2013-12-21 07:31:18 PST 
(In reply to comment #13)
> This looks like the root cause:
> 
> http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r160955%20(1283)/fast/events/nested-event-remove-node-crash-crash-log.txt
> 
> 12  com.apple.WebCore                 0x00000001079381c8 WebCore::Style::resolveTree(WebCore::Document&, WebCore::Style::Change) + 520 (StyleResolveTree.cpp:880)
> 13  com.apple.WebCore                 0x0000000107c4ace6 WebCore::Document::recalcStyle(WebCore::Style::Change) + 438 (Document.cpp:1760)
> 14  com.apple.WebCore                 0x0000000107c474df WebCore::Document::updateStyleIfNeeded() + 431 (Document.cpp:1809)
> 15  com.apple.WebCore                 0x00000001080d16df WebCore::HTMLElement::supportsFocus() const + 111 (HTMLElement.cpp:667)
> 16  com.apple.WebCore                 0x00000001077169c6 WebCore::AccessibilityNodeObject::canSetFocusAttribute() const + 214 (AccessibilityNodeObject.cpp:1985)
> 17  com.apple.WebCore                 0x0000000107723839 WebCore::AccessibilityRenderObject::inheritsPresentationalRole() const + 41 (AccessibilityRenderObject.cpp:2625)
> 18  com.apple.WebCore                 0x0000000107722d95 WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const + 325 (AccessibilityRenderObject.cpp:1183)
> 19  com.apple.WebCore                 0x000000010771e84f WebCore::AccessibilityObject::accessibilityIsIgnored() const + 191 (AccessibilityObject.cpp:2133)
> 20  com.apple.WebCore                 0x00000001077080cd WebCore::AccessibilityObject::notifyIfIgnoredValueChanged() + 29 (AccessibilityObject.cpp:2033)
> 21  com.apple.WebCore                 0x0000000107708dc8 WebCore::AXObjectCache::recomputeIsIgnored(WebCore::RenderObject*) + 56 (AXObjectCache.cpp:864)
> 22  com.apple.WebCore                 0x0000000108be0359 WebCore::RenderBlock::deleteLines() + 73 (RenderBlock.cpp:932)
> 23  com.apple.WebCore                 0x0000000109462ea2 WebCore::RenderBlockFlow::deleteLines() + 450 (RenderBlockFlow.cpp:1827)
> 24  com.apple.WebCore                 0x0000000108be10cd WebCore::RenderBlock::removeChild(WebCore::RenderObject&) + 1709 (RenderBlock.cpp:1191)
> 25  com.apple.WebCore                 0x0000000108d91e06 WebCore::RenderObject::removeFromParent() + 70 (RenderObject.cpp:187)
> 26  com.apple.WebCore                 0x0000000108d9c2f3 WebCore::RenderObject::willBeDestroyed() + 163 (RenderObject.cpp:1865)
> 27  com.apple.WebCore                 0x0000000108e77647 WebCore::RenderText::willBeDestroyed() + 167 (RenderText.cpp:284)
> 28  com.apple.WebCore                 0x0000000108d9c8bd WebCore::RenderObject::destroy() + 29 (RenderObject.cpp:1985)
> 29  com.apple.WebCore                 0x0000000108d9c892 WebCore::RenderObject::destroyAndCleanupAnonymousWrappers() + 274 (RenderObject.cpp:1975)
> 30  com.apple.WebCore                 0x0000000107937012 WebCore::Style::detachTextRenderer(WebCore::Text&) + 50 (StyleResolveTree.cpp:414)
> 31  com.apple.WebCore                 0x0000000107938661 WebCore::Style::detachChildren(WebCore::ContainerNode&, WebCore::Style::DetachType) + 81 
> 
> We are entering style resolve from the middle of render tree destruction.

Two thoughts:
   Does  WebCore::HTMLElement::supportsFocus()   really need to update the style calculations?
   
   It's possible this work
      WebCore::AXObjectCache::recomputeIsIgnored
   can be pended on a timer to run during the next runloop iteration
Comment 15 Antti Koivisto 2013-12-21 11:58:42 PST 
Looks like https://trac.webkit.org/r160966 fixed this.
Comment 16 Antti Koivisto 2013-12-21 12:24:40 PST 
Rolled out the hack fix in https://trac.webkit.org/r160968