WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
248926
AX ITM: Fix for tests concerning CaretBrowsing and PreventKeyboardEventDispatch in isolated tree mode.
https://bugs.webkit.org/show_bug.cgi?id=248926
Summary
AX ITM: Fix for tests concerning CaretBrowsing and PreventKeyboardEventDispat...
Andres Gonzalez
Reported
2022-12-07 19:53:01 PST
Fix for 4 tests in ITM.
Attachments
Patch
(35.65 KB, patch)
2022-12-07 19:59 PST
,
Andres Gonzalez
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2022-12-07 19:53:12 PST
<
rdar://problem/103101568
>
Andres Gonzalez
Comment 2
2022-12-07 19:59:58 PST
Created
attachment 463931
[details]
Patch
Tyler Wilcock
Comment 3
2022-12-07 20:41:55 PST
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.
Andres Gonzalez
Comment 4
2022-12-08 05:25:35 PST
(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.
EWS
Comment 5
2022-12-09 10:48:01 PST
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]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug