Bug 234739 - AX: web process crash with isolated tree mode enabled
Summary: AX: web process crash with isolated tree mode enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-29 06:58 PST by Carlos Garcia Campos
Modified: 2022-01-03 01:21 PST (History)
10 users (show)

See Also:


Attachments
Patch (5.15 KB, patch)
2021-12-29 07:04 PST, Carlos Garcia Campos
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2021-12-29 06:58:54 PST
#0  0x00007f6a18cad3fc in WebCore::AccessibilityObjectAtspi::attach(WebCore::AXCoreObject*) () from libwebkit2gtk-4.1.so.0
#1  0x00007f6a18ceb0ff in WebCore::AXIsolatedTree::createSubtree(WebCore::AXCoreObject&, WTF::ObjectIdentifier<WebCore::AXIDType>, bool) ()
   from libwebkit2gtk-4.1.so.0
#2  0x00007f6a18ceb449 in WebCore::AXIsolatedTree::generateSubtree(WebCore::AXCoreObject&, WebCore::AXCoreObject*, bool) [clone .part.0] ()
   from libwebkit2gtk-4.1.so.0
#3  0x00007f6a18cebf2c in WebCore::AXIsolatedTree::updateChildren(WebCore::AXCoreObject&) () from libwebkit2gtk-4.1.so.0
#4  0x00007f6a18c468bf in WebCore::AXObjectCache::updateIsolatedTree(WTF::Vector<std::pair<WTF::RefPtr<WebCore::AXCoreObject, WTF::RawPtrTraits<WebCore::AXCoreObject>, WTF::DefaultRefDerefTraits<WebCore::AXCoreObject> >, WebCore::AXObjectCache::AXNotification>, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&) () from libwebkit2gtk-4.1.so.0
#5  0x00007f6a18c46afb in WebCore::AXObjectCache::notificationPostTimerFired() () from libwebkit2gtk-4.1.so.0
#6  0x00007f6a198b597f in WebCore::ThreadTimers::sharedTimerFiredInternal() [clone .part.0] () from libwebkit2gtk-4.1.so.0
#7  0x00007f6a1674bd15 in WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)#1}::_FUN(void*) () from libjavascriptcoregtk-4.1.so.0
#8  0x00007f6a1674c2af in WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)#1}::_FUN(_GSource*, int (*)(void*), void*) () from libjavascriptcoregtk-4.1.so.0
#9  0x00007f6a16b2e5c4 in g_main_dispatch (context=0x55bbde8f2260) at ../glib/gmain.c:3381
#10 g_main_context_dispatch (context=0x55bbde8f2260) at ../glib/gmain.c:4099
#11 0x00007f6a16b2e928 in g_main_context_iterate (context=0x55bbde8f2260, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4175
#12 0x00007f6a16b2ec03 in g_main_loop_run (loop=0x55bbdea74600) at ../glib/gmain.c:4373
#13 0x00007f6a1674c3d0 in WTF::RunLoop::run() () from libjavascriptcoregtk-4.1.so.0
#14 0x00007f6a1816f5ff in int WebKit::AuxiliaryProcessMain<WebKit::WebProcessMainGtk>(int, char**) () from libwebkit2gtk-4.1.so.0
#15 0x00007f6a16f877ed in __libc_start_main (main=0x55bbdcef46a0 <main>, argc=3, argv=0x7ffc8a5b7ae8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7ffc8a5b7ad8) at ../csu/libc-start.c:332
#16 0x000055bbdcef46da in _start ()

The wrapper we try to attach has already been deleted. This is what happens:

1.- children changed notification
2.- AXIsolatedTree::updateChildren() calls generateSubtree() to add a new child.
3.- generateSubtree() calls createSubtree() that creates the AXIsolatedObject
4.- AXIsolatedObject::initializeAttributeData() calls AccessibilityObject::computedLabel() to initialize the computed label property
5.- AccessibilityObject::computedLabel() calls updateBackingStore that triggers a layout
6.- The layout ends up removing the new node from the cache, causing the wrapper to be detached and destroyed.
7.- AXIsolatedObject::create() finishes and now axObject.wrapper() is nullptr when attachPlatformWrapper is called.

I think we can just get rid of computedLabel for isolated objects, since that is only used by the inspector that uses the AccessibilityObject directly.
Comment 1 Radar WebKit Bug Importer 2021-12-29 06:59:09 PST
<rdar://problem/86983058>
Comment 2 Carlos Garcia Campos 2021-12-29 07:04:48 PST
Created attachment 448070 [details]
Patch
Comment 3 chris fleizach 2021-12-29 08:26:06 PST
Comment on attachment 448070 [details]
Patch

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

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:804
> +    return { };

Can you add a comment explaining why we shouldn’t reach here
Comment 4 Carlos Garcia Campos 2021-12-30 07:13:25 PST
(In reply to chris fleizach from comment #3)
> Comment on attachment 448070 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=448070&action=review
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:804
> > +    return { };
> 
> Can you add a comment explaining why we shouldn’t reach here

Sure, I'll add it before landing.
Comment 5 Carlos Garcia Campos 2021-12-30 07:20:16 PST
I've noticed a lot more layout tests failures with isolated tree mode enabled with this patch. The updateBackingStore call while creating isolated objects caused a layout + updateChildrenIfNeeded that made the isolated tree to be fully populated earlier. I still think the early layout is not desirable when creating the isolated tree, but had the side effect of having the full tree available soon. This will require more a11y tests to be async to pass with isolated tree.
Comment 6 Carlos Garcia Campos 2022-01-03 01:21:34 PST
Committed r287533 (245668@trunk): <https://commits.webkit.org/245668@trunk>