Bug 96932 - [GTK] [Stable] Infinite recursion in WebCore::AXObjectCache::getOrCreate
Summary: [GTK] [Stable] Infinite recursion in WebCore::AXObjectCache::getOrCreate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Critical
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-17 10:43 PDT by Martin Robinson
Modified: 2012-09-21 12:40 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.98 KB, patch)
2012-09-19 22:31 PDT, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Patch (4.92 KB, patch)
2012-09-20 21:21 PDT, Joanmarie Diggs
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2012-09-17 10:43:03 PDT
Here's the backtrace:


#98746 0x00007ffff5f0eb6e in WebCore::AccessibilityRenderObject::create(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98747 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98748 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98749 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98750 0x00007ffff5f10f43 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98751 0x00007ffff5f14dc0 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98752 0x00007ffff5efe022 in WebCore::AccessibilityNodeObject::remapAriaRoleDueToParent(WebCore::AccessibilityRole) const () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98753 0x00007ffff5efe0ac in WebCore::AccessibilityNodeObject::determineAriaRoleAttribute() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98754 0x00007ffff5f11dbc in WebCore::AccessibilityRenderObject::determineAccessibilityRole() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98755 0x00007ffff5efd98d in WebCore::AccessibilityNodeObject::init() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98756 0x00007ffff5f0eb6e in WebCore::AccessibilityRenderObject::create(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98757 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98758 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98759 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98760 0x00007ffff5f10f43 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98761 0x00007ffff5f14dc0 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98762 0x00007ffff5efe022 in WebCore::AccessibilityNodeObject::remapAriaRoleDueToParent(WebCore::AccessibilityRole) const () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98763 0x00007ffff5efe0ac in WebCore::AccessibilityNodeObject::determineAriaRoleAttribute() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98764 0x00007ffff5f11dbc in WebCore::AccessibilityRenderObject::determineAccessibilityRole() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98765 0x00007ffff5efd98d in WebCore::AccessibilityNodeObject::init() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98766 0x00007ffff5f0eb6e in WebCore::AccessibilityRenderObject::create(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98767 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98768 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98769 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()

#98773 0x00007ffff5efe0ac in WebCore::AccessibilityNodeObject::determineAriaRoleAttribute() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98774 0x00007ffff5f11dbc in WebCore::AccessibilityRenderObject::determineAccessibilityRole() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98775 0x00007ffff5efd98d in WebCore::AccessibilityNodeObject::init() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98776 0x00007ffff5f0eb6e in WebCore::AccessibilityRenderObject::create(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98777 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98778 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98779 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98780 0x00007ffff5f10f43 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98781 0x00007ffff5f14dc0 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98782 0x00007ffff5efe022 in WebCore::AccessibilityNodeObject::remapAriaRoleDueToParent(WebCore::AccessibilityRole) const () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98783 0x00007ffff5efe0ac in WebCore::AccessibilityNodeObject::determineAriaRoleAttribute() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98784 0x00007ffff5f11dbc in WebCore::AccessibilityRenderObject::determineAccessibilityRole() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98785 0x00007ffff5efd98d in WebCore::AccessibilityNodeObject::init() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98786 0x00007ffff5f0eb6e in WebCore::AccessibilityRenderObject::create(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98787 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98788 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98789 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98790 0x00007ffff5f10f43 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98791 0x00007ffff5f14dc0 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98792 0x00007ffff5efe022 in WebCore::AccessibilityNodeObject::remapAriaRoleDueToParent(WebCore::AccessibilityRole) const () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
Comment 1 Martin Robinson 2012-09-17 10:59:26 PDT
Oops. Here's the bottom of the stack:

#98817 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98818 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98819 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98820 0x00007ffff5f10f43 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98821 0x00007ffff5f14dc0 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98822 0x00007ffff5efe022 in WebCore::AccessibilityNodeObject::remapAriaRoleDueToParent(WebCore::AccessibilityRole) const () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98823 0x00007ffff5efe0ac in WebCore::AccessibilityNodeObject::determineAriaRoleAttribute() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98824 0x00007ffff5f11dbc in WebCore::AccessibilityRenderObject::determineAccessibilityRole() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98825 0x00007ffff5efd98d in WebCore::AccessibilityNodeObject::init() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98826 0x00007ffff5f0eb6e in WebCore::AccessibilityRenderObject::create(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98827 0x00007ffff5f1c61c in WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98828 0x00007ffff5f02cbf in WebCore::AccessibilityObject::firstAnonymousBlockChild() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98829 0x00007ffff6e4f326 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98830 0x00007ffff5f10f43 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98831 0x00007ffff5f14dc0 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98832 0x00007ffff5f13d0b in WebCore::AccessibilityRenderObject::addChildren() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98833 0x00007ffff5f023cd in WebCore::AccessibilityObject::children() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98834 0x00007ffff5f13dba in WebCore::AccessibilityRenderObject::addChildren() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98835 0x00007ffff5f023cd in WebCore::AccessibilityObject::children() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98836 0x00007ffff6e5b02c in webkitAccessibleGetNChildren(_AtkObject*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#98837 0x00007fffeef0c44c in append_children (traversal=0x6dd2c0, accessible=0x2b75770) at accessible-cache.c:235
#98838 add_pending_items (data=<optimized out>) at accessible-cache.c:291
#98839 0x00007ffff3c893e3 in g_main_dispatch (context=0x704d40) at gmain.c:2715
#98840 g_main_context_dispatch (context=0x704d40) at gmain.c:3219
#98841 0x00007ffff3c89730 in g_main_context_iterate (dispatch=1, block=<optimized out>, context=0x704d40, 
    self=<optimized out>) at gmain.c:3290
---Type <return> to continue, or q <return> to quit---
#98842 g_main_context_iterate (context=0x704d40, block=<optimized out>, dispatch=1, self=<optimized out>)
    at gmain.c:3227
#98843 0x00007ffff3c897f4 in g_main_context_iteration (context=0x704d40, may_block=1) at gmain.c:3351
#98844 0x00007ffff42538b4 in g_application_run (application=0x808000, argc=<optimized out>, argv=0x7fffffffd668)
    at gapplication.c:1607
Comment 2 Martin Robinson 2012-09-17 11:07:04 PDT
This seems to be the crux of the problem:

WebCore::AccessibilityObject::firstAnonymousBlockChild calls
WebCore::AXObjectCache::getOrCreate calls
WebCore::AccessibilityRenderObject::create calls
WebCore::AccessibilityNodeObject::init calls
WebCore::AccessibilityRenderObject::determineAccessibilityRole calls
WebCore::AccessibilityNodeObject::determineAriaRoleAttribute calls
WebCore::AccessibilityNodeObject::remapAriaRoleDueToParent calls
WebCore::AccessibilityRenderObject::accessibilityIsIgnored calls
WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase calls
WebCore::AccessibilityObject::accessibilityPlatformIncludesObject calls
WebCore::AccessibilityObject::firstAnonymousBlockChild
Comment 3 Joanmarie Diggs 2012-09-17 13:29:52 PDT
I'm working on this.

Rather than merely back out the associated commit (which would result in needed objects getting ignored by the core a11y code), worst case scenario is that we always include objects of ParagraphRole and DivRole for now. That would eliminate the recursion and the price would not be too high: the occasional extra paragraph/div.

In the meantime, if you can find the original test case for the purpose of a new Layout Test, that would be great as I have not encountered this crash and am not yet familiar with the ARIA code that is getting called enough to guess the exact scenario.
Comment 4 Joanmarie Diggs 2012-09-19 22:31:06 PDT
Created attachment 164839 [details]
Patch
Comment 5 Joanmarie Diggs 2012-09-19 22:34:04 PDT
Martin, sorry for the delay on this. The F18-related crashers and inability to run any WebKitGtk test wound up requiring downgrades, re-installations, etc. to F17 :(

Anyhoo, the attached patch solves the problem and includes a new test based on the Google+ crash. Thanks for that! Please review.
Comment 6 Martin Robinson 2012-09-20 12:21:59 PDT
It's always good to make tests as simple and minimal as possible, so please consider this alternate test:

<html>
</body>

<div>
    The presence of remapped ARIA objects should not crash the test harness.
    <a role="option"></a>
</div>

<script>
    if (window.testRunner)
        window.testRunner.dumpAsText();
    document.body.focus();
</script>

</body>
</html>
Comment 7 Martin Robinson 2012-09-20 12:22:19 PDT
By the way, thanks for fixing this!
Comment 8 Joanmarie Diggs 2012-09-20 13:26:45 PDT
(In reply to comment #6)
> It's always good to make tests as simple and minimal as possible, so please consider this alternate test:
> 
> <html>
> </body>
> 
> <div>
>     The presence of remapped ARIA objects should not crash the test harness.
>     <a role="option"></a>
> </div>
> 
> <script>
>     if (window.testRunner)
>         window.testRunner.dumpAsText();
>     document.body.focus();
> </script>
> 
> </body>
> </html>

Does that reliably crash for you? I tried all sorts of simple ways to make the crash occur 100% of the time, but could not:

* Just loading the test and giving the body focus
* Synthesizing a click on the various elements
* Directly accessing a specific accessible (and I tried them all)

The reason I went with a full hierarchy dump is that the one way I can reliably repro the crash outside of the test harness is to view the page and drill down the accessible tree in Accerciser. So that is what I did. For me the attached test is the ONLY thing which will crash 100% of the time.
Comment 9 Martin Robinson 2012-09-20 14:02:25 PDT
Comment on attachment 164839 [details]
Patch

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

> Source/WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp:95
> +        for (RenderObject* r = renderer()->firstChild(); r; r = r->nextSibling()) {
> +            if (r->isAnonymousBlock())
> +                return IncludeObject;

I notice here that the check for child->isLink() is gone now. Was it unimportant before or was it replaced by another check? I'm also a bit confused by the change in logic. I'm probably reading it incorrectly.

My understanding:
Before: If the child is a link or the first child isn't an anonymous block, include the object.
Now: If any child is an anonymous block, include the object.

Not sure I understand that fully.
Comment 10 Joanmarie Diggs 2012-09-20 19:12:04 PDT
(In reply to comment #9)
> (From update of attachment 164839 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164839&action=review
> 
> > Source/WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp:95
> > +        for (RenderObject* r = renderer()->firstChild(); r; r = r->nextSibling()) {
> > +            if (r->isAnonymousBlock())
> > +                return IncludeObject;
> 
> I notice here that the check for child->isLink() is gone now. Was it unimportant before or was it replaced by another check? I'm also a bit confused by the change in logic. I'm probably reading it incorrectly.
> 
> My understanding:
> Before: If the child is a link or the first child isn't an anonymous block, include the object.
> Now: If any child is an anonymous block, include the object.
> 
> Not sure I understand that fully.

The whole Accessible Anonymous Block (is that an oxymoron?) situation makes my eyes cross. So I am sure I'm doing a poor job of explaining.

The change in logic is admittedly trading a bit of certainty for probability in order to achieve stability. FWIW there are already regression tests which I added back when I was initially sorting all of this out to ensure the critical problems don't creep back in. I think we'll find a new edge case here and there, but we can deal with those when they arise. Besides I'll take hypothetical edge case over a very real crasher any day of the week. ;)

Anyhoo.... 

Given a Para/Div which has at least one anonymous block child, we want to include the Para/Div.
Exception 1: Nested anonymous blocks are a mess. Defer to WebCore.
Exception 2: The first child of the anonymous block is a link. In that case we also want to include the Para/Div because that Para/Div is the HTML element which contains the link.

The old/current/crashy logic was:
  * If none of Para/Div's children is an anonymous block: Defer to WebCore.
  * If the first child of the first anonymous block found is a link: include Para/Div.
  * If the first child of the first anonymous block has no anonymous block children, we are not in a nested anonymous block situation: Include Para/Div.
  * Otherwise fall back on the default WebCore behavior to sort out the nested anonymous block mess.

The new/proposed logic is:
  * If we're pretty confident [1] none of Para/Div's children is an anonymous block: Defer to WebCore.
  * If Para/Div's parent is an anonymous block we may [2] have a mess of nested anonymous blocks: Defer to WebCore.
  * We are not in a nested anonymous block situation. Thus if Para/Div has any anonymous block children: include Para/Div. [3]

[1] What really matters / has been problematic is Anonymous Blocks *which contain text* getting included at the expense of ignoring the parent Para/Div. If there is no text under the Para/Div element at all, there cannot be Anonymous Blocks with text as immediate children.

[2] Then again, we may not. There's a chance for an unaddressed edge case here. Haven't found it yet. And perhaps WebCore is just doing the RightThing(tm) for us in these cases.

[3] The Para/Div->Anon Block->Link case gets addressed here because we include all Para/Div->Anon Block cases as long as the parent of the Para/Div is not also an Anon Block. And perhaps we are missing another edge case here. But falling back to WebCore to sort out this mess is not, to me, a huge risk.
Comment 11 Joanmarie Diggs 2012-09-20 21:21:49 PDT
Created attachment 165043 [details]
Patch
Comment 12 WebKit Review Bot 2012-09-21 12:39:58 PDT
Comment on attachment 165043 [details]
Patch

Clearing flags on attachment: 165043

Committed r129246: <http://trac.webkit.org/changeset/129246>
Comment 13 WebKit Review Bot 2012-09-21 12:40:01 PDT
All reviewed patches have been landed.  Closing bug.