Bug 138905 – AX: com.apple.WebKit.WebContent crashed at WebCore: WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Subtype: KERN_INVALID_ADDRESS at 0x00000008
Triggered by Thread:  0

Filtered syslog:
None found

Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed:
0   WebCore                       	0x33714618 WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const + 180 (RenderObject.h:160)
1   WebCore                       	0x337061e8 WebCore::AccessibilityObject::accessibilityIsIgnored() const + 212 (AccessibilityObject.cpp:2517)
2   WebCore                       	0x33703c40 WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) + 1364 (AXObjectCache.cpp:435)
3   WebCore                       	0x33711f70 WebCore::AccessibilityObject::isARIAHidden() const + 188 (AccessibilityObject.cpp:2468)
4   WebCore                       	0x337144da WebCore::AccessibilityRenderObject::defaultObjectInclusion() const + 94 (AccessibilityObject.cpp:2489)
5   WebCore                       	0x33714578 WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const + 20 (AccessibilityRenderObject.cpp:1169)
6   WebCore                       	0x337061e8 WebCore::AccessibilityObject::accessibilityIsIgnored() const + 212 (AccessibilityObject.cpp:2517)
7   WebCore                       	0x33703c40 WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) + 1364 (AXObjectCache.cpp:435)
8   WebCore                       	0x33711f70 WebCore::AccessibilityObject::isARIAHidden() const + 188 (AccessibilityObject.cpp:2468)
9   WebCore                       	0x337144da WebCore::AccessibilityRenderObject::defaultObjectInclusion() const + 94 (AccessibilityObject.cpp:2489)
10  WebCore                       	0x33714578 WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const + 20 (AccessibilityRenderObject.cpp:1169)
11  WebCore                       	0x337061e8 WebCore::AccessibilityObject::accessibilityIsIgnored() const + 212 (AccessibilityObject.cpp:2517)
12  WebCore                       	0x33703c40 WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) + 1364 (AXObjectCache.cpp:435)
13  WebCore                       	0x33711f70 WebCore::AccessibilityObject::isARIAHidden() const + 188 (AccessibilityObject.cpp:2468)
14  WebCore                       	0x337144da WebCore::AccessibilityRenderObject::defaultObjectInclusion() const + 94 (AccessibilityObject.cpp:2489)
15  WebCore                       	0x33714578 WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const + 20 (AccessibilityRenderObject.cpp:1169)
16  WebCore                       	0x337061e8 WebCore::AccessibilityObject::accessibilityIsIgnored() const + 212 (AccessibilityObject.cpp:2517)
17  WebCore                       	0x33703c40 WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) + 1364 (AXObjectCache.cpp:435)
18  WebCore                       	0x33704432 WebCore::AXObjectCache::textChanged(WebCore::RenderObject*) + 6 (AXObjectCache.cpp:625)
19  WebCore                       	0x33e1f1ea WebCore::RenderText::setText(WTF::String const&, bool) + 274 (RenderText.cpp:1109)
20  WebCore                       	0x3360bc50 WebCore::RenderMenuList::setText(WTF::String const&) + 76 (RenderMenuList.cpp:296)
21  WebCore                       	0x3360bae0 WebCore::RenderMenuList::setTextFromOption(int) + 376 (RenderMenuList.cpp:287)
22  WebCore                       	0x3360ca88 WebCore::HTMLSelectElement::selectOption(int, unsigned int) + 360 (HTMLSelectElement.cpp:894)
23  WebCore                       	0x33b55618 WebCore::setJSHTMLSelectElementSelectedIndex(JSC::ExecState*, JSC::JSObject*, long long, long long) + 260 (JSHTMLSelectElement.cpp:723)
24  JavaScriptCore                	0x28170b3a JSC::callCustomSetter(JSC::ExecState*, JSC::JSValue, JSC::JSObject*, JSC::JSValue, JSC::JSValue) + 34 (CustomGetterSetter.cpp:44)
25  JavaScriptCore                	0x28009622 JSC::JSObject::put(JSC::JSCell*, JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&) + 682 (JSObject.cpp:383)
26  WebCore                       	0x336af89c WebCore::JSHTMLSelectElement::put(JSC::JSCell*, JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&) + 64 


<rdar://problem/19036181>

[chris fleizach]

Created attachment 241928 [details]
patch

[Mario Sanchez Prada]

Comment on attachment 241928 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197
> +    if (m_renderer && ancestorsOfType<RenderMenuList>(*m_renderer).first())

You early returned on !m_renderer, so I don't think you need this extra check, unless any of the previous calls cause any side effect on the renderer (which should not be the case)

[chris fleizach]

(In reply to comment #2)
> Comment on attachment 241928 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241928&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197
> > +    if (m_renderer && ancestorsOfType<RenderMenuList>(*m_renderer).first())
> 
> You early returned on !m_renderer, so I don't think you need this extra
> check, unless any of the previous calls cause any side effect on the
> renderer (which should not be the case)

I think there is a chance that m_renderer may be set to nil by something else in the method (descendantOfBarrenChildren())? It's possible that going up the render tree is causing that. I don't have any evidence of that and I've never seen it happen, but the crash report leaves open that interpretation. 

do you think we should keep both checks just in case?

[Mario Sanchez Prada]

Comment on attachment 241928 [details]
patch

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

>>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197
>>> +    if (m_renderer && ancestorsOfType<RenderMenuList>(*m_renderer).first())
>> 
>> You early returned on !m_renderer, so I don't think you need this extra check, unless any of the previous calls cause any side effect on the renderer (which should not be the case)
> 
> I think there is a chance that m_renderer may be set to nil by something else in the method (descendantOfBarrenChildren())? It's possible that going up the render tree is causing that. I don't have any evidence of that and I've never seen it happen, but the crash report leaves open that interpretation. 
> 
> do you think we should keep both checks just in case?

I guess that the "fearful me" agree on that it would perhaps be worth leaving the two checks, just in case.

Anyway, as this is an speculative fix, either option (to leave both checks or only one) will be a gamble, so I think I lean more towards the safest one.

[WebKit Commit Bot]

Comment on attachment 241928 [details]
patch

Clearing flags on attachment: 241928

Committed r176484: <http://trac.webkit.org/changeset/176484>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.