Bug 257894 – AX: Make the generation and update of isolated subtrees asynchronous.

• Status: NEW
• Component: Accessibility

[Andres Gonzalez]

This synchronous method contributes to Safari becoming irresponsive to AT requests.

[Radar WebKit Bug Importer]

<rdar://problem/110532050>

[Andres Gonzalez]

Created attachment 466651 [details]
Patch

[Tyler Wilcock]

Comment on attachment 466651 [details]
Patch

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

> Source/WebCore/accessibility/AXObjectCache.cpp:902
> +    auto tree = AXIsolatedTree::create(*this, [this] () {

When capturing `this` in the completion handler, we should be storing it in a `WeakPtr` or `CheckedPtr` (or RefPtr, but currently you can't make a ref-counted AXObjectCache) to avoid a UAF if the cache gets deleted before the completion handler runs.

> Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:362
> +    auto now = MonotonicTime::now();

I wonder if this should be called something like "start" or "startTime". It might make the comparison below more clear (which currently subtracts "MonotonicTome::now" from "now").

[Andres Gonzalez]

Created attachment 466669 [details]
Patch

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #3)
> Comment on attachment 466651 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=466651&action=review
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:902
> > +    auto tree = AXIsolatedTree::create(*this, [this] () {
> 
> When capturing `this` in the completion handler, we should be storing it in
> a `WeakPtr` or `CheckedPtr` (or RefPtr, but currently you can't make a
> ref-counted AXObjectCache) to avoid a UAF if the cache gets deleted before
> the completion handler runs.

Done.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:362
> > +    auto now = MonotonicTime::now();
> 
> I wonder if this should be called something like "start" or "startTime". It
> might make the comparison below more clear (which currently subtracts
> "MonotonicTome::now" from "now").

Renamed.

Also added in the new revision the check for Accessibility::waitingForMainThread.

[Andres Gonzalez]

Created attachment 468261 [details]
Patch

[Andres Gonzalez]

Created attachment 468262 [details]
Patch

Updating expectation for glib test.