| Summary: | AX: AXIsolatedTree::children() should not re-initialize m_children unless they've changed | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tyler Wilcock <tyler_w> | ||||
| Component: | Accessibility | Assignee: | Tyler Wilcock <tyler_w> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | Other | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Tyler Wilcock
2023-10-09 20:30:30 PDT
Created attachment 468141 [details]
Patch
Comment on attachment 468141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468141&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:463 > + m_childrenIDs = WTFMove(ids); Can we avoid dirty children if the IDs are same in same order? (In reply to chris fleizach from comment #3) > Comment on attachment 468141 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468141&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:463 > > + m_childrenIDs = WTFMove(ids); > > Can we avoid dirty children if the IDs are same in same order? I thought about this too...but ended up deciding against it since it forces an O(n) iteration for what is likely a very uncommon case. Additionally, this proposed iteration would have to happen while we hold the m_changeLogLock, potentially increasing lock contention. (In reply to Tyler Wilcock from comment #2) > Created attachment 468141 [details] > Patch Are we covering the unconnected objects case? We don't update m_pendingchildrenUpdates for those objects. (In reply to Andres Gonzalez from comment #5) > (In reply to Tyler Wilcock from comment #2) > > Created attachment 468141 [details] > > Patch > > Are we covering the unconnected objects case? We don't update > m_pendingchildrenUpdates for those objects. I don't think that's necessary here. The only time we need to mark children as dirty is when AXIsolatedObject::m_childrenIDs changes, which addUnconnectedNode doesn't do. Committed 269170@main (4d35a6d87939): <https://commits.webkit.org/269170@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468141 [details]. |