| Summary: | AX ITM: Fix for tests concerning CaretBrowsing and PreventKeyboardEventDispatch in isolated tree mode. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andres Gonzalez <andresg_22> | ||||
| Component: | Accessibility | Assignee: | Andres Gonzalez <andresg_22> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, tyler_w, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Andres Gonzalez
2022-12-07 19:53:01 PST
Created attachment 463931 [details]
Patch
Comment on attachment 463931 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463931&action=review Nice work! > Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:101 > + setProperty(AXPropertyName::PreventKeyboardDOMEventDispatch, value); Is it required to set this property on the main-thread? Or could we do it on the secondary thread? I think the latter option would be better to yield the main-thread as soon as possible. And as a follow-up, performFunctionOnMainThread calls into `callOnMainThreadAndWait`. Would it be safe to use something else that calls into `callOnMainThread` or `ensureOnMainThread` (which are async) here to free up the secondary thread immediately for serving requests? Same comment for setCaretBrowsingEnabled. (In reply to Tyler Wilcock from comment #3) > Comment on attachment 463931 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=463931&action=review > > Nice work! > > > Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:101 > > + setProperty(AXPropertyName::PreventKeyboardDOMEventDispatch, value); > > Is it required to set this property on the main-thread? Or could we do it on > the secondary thread? I think the latter option would be better to yield the > main-thread as soon as possible. > > And as a follow-up, performFunctionOnMainThread calls into > `callOnMainThreadAndWait`. Would it be safe to use something else that calls > into `callOnMainThread` or `ensureOnMainThread` (which are async) here to > free up the secondary thread immediately for serving requests? Same comment > for setCaretBrowsingEnabled. setProperty accesses m_propertyMap which is accessed in both threads. So we need to call it on each thread when the other is blocked. getOrRetrievePropertyValue is currently calling setProperty on the main thread, so that's why I opted for doing so in this case. Maybe we can move setProperty to the AX thread including also the call in getOrRetrievePropertyValue, except for the initialization of the object, but that would be a separate change. At this point the safest option is to do the same as getOrRetrievePropertyValue. Given this, callOnMainThreadAndWait is required to block the AX thread while we call setProperty on the main thread. Committed 257638@main (f198a4dc27e9): <https://commits.webkit.org/257638@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 463931 [details]. |