Bug 243456 – AX ITM: Avoid dispatching AXIsolatedObject methods to the main thread when it is busy updating the isolated tree.

• Component: Accessibility
• Status: NEW

[Andres Gonzalez]

This causes VoiceOver to become irresponsive.

[Radar WebKit Bug Importer]

<rdar://problem/97982075>

[Andres Gonzalez]

Created attachment 461361 [details]
Patch

[Darin Adler]

Comment on attachment 461361 [details]
Patch

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

I didn’t get a chance to review the whole thing, but a couple comments.

> Source/WebCore/accessibility/AccessibilityObjectInterface.h:1695
> +T retrieveValueFromMainThreadWithTimeout(U&& function, Seconds&& timeout)

Since Seconds is a class wrapping a double, we can just use Seconds, we don’t need to use Seconds&&.

> Source/WebCore/accessibility/AccessibilityObjectInterface.h:1708
> +    while (!done && !hasElapsed(elapseTime)) { }

This kind of loop is not an efficient way to have one thread wait on another. Surely there is a better idiom for such things using locks, mutexes, or semaphores of some kind.

[Andres Gonzalez]

Created attachment 461559 [details]
Patch

Extended the check for whether to dispatch to the main thread to all AXIsolatedObject methods that were using Accessibility::retrieveValueFromMainThread.

[Andres Gonzalez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 461361 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=461361&action=review
> 
> I didn’t get a chance to review the whole thing, but a couple comments.
> 
> > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1695
> > +T retrieveValueFromMainThreadWithTimeout(U&& function, Seconds&& timeout)
> 
> Since Seconds is a class wrapping a double, we can just use Seconds, we
> don’t need to use Seconds&&.

Fixed.
> 
> > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1708
> > +    while (!done && !hasElapsed(elapseTime)) { }
> 
> This kind of loop is not an efficient way to have one thread wait on
> another. Surely there is a better idiom for such things using locks,
> mutexes, or semaphores of some kind.

Yes, still working on that. I'm getting deadlocks using BinarySemaphore that I'm still debugging. I'm keeping this for now so that it is testable but not the final version yet. Thanks!