RESOLVED FIXED264045
AX: Fix thread safety issue in AXIsolatedTree::setSelectedTextMarkerRange 
https://bugs.webkit.org/show_bug.cgi?id=264045
Summary AX: Fix thread safety issue in AXIsolatedTree::setSelectedTextMarkerRange
Joshua Hoffman
Reported 2023-11-01 10:26:09 PDT
We need to hold a lock when modifying m_selectedTextMarkerRange on the main thread (modified on the main thread in AXIsolatedTree::setSelectedTextMarkerRange).
Attachments
Patch (2.75 KB, patch)
2023-11-01 10:34 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (3.07 KB, patch)
2023-11-01 23:01 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2023-11-01 10:26:19 PDT
<rdar://problem/117803232>
Joshua Hoffman
Comment 2 2023-11-01 10:34:01 PDT
Created attachment 468448 [details] Patch
Andres Gonzalez
Comment 3 2023-11-01 19:19:57 PDT
(In reply to Joshua Hoffman from comment #2) > Created attachment 468448 [details] > Patch diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp index 057f683c009d..e2d551382ff4 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp @@ -910,6 +910,24 @@ void AXIsolatedTree::setFocusedNodeID(AXID axID) m_pendingFocusedNodeID = axID; } +AXTextMarkerRange AXIsolatedTree::selectedTextMarkerRange() +{ + AXTRACE("AXIsolatedTree::selectedTextMarkerRange"_s); + Locker locker { m_changeLogLock }; + return m_selectedTextMarkerRange; +} + AG: extra blank line. + +void AXIsolatedTree::setSelectedTextMarkerRange(AXTextMarkerRange range) +{ + AXTRACE("AXIsolatedTree::setSelectedTextMarkerRange"_s); + ASSERT(isMainThread()); + + Locker locker { m_changeLogLock }; + m_selectedTextMarkerRange = range; +} + AG: extra blank line. + void AXIsolatedTree::labelCreated(AccessibilityObject& axObject) { ASSERT(axObject.isLabel()); diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h index ebc80fa48083..78b51ca89317 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h @@ -321,8 +321,8 @@ public: // Use only if the s_storeLock is already held like in findAXTree. WEBCORE_EXPORT OptionSet<ActivityState> lockedPageActivityState() const; - AXTextMarkerRange selectedTextMarkerRange() { return m_selectedTextMarkerRange; }; - void setSelectedTextMarkerRange(AXTextMarkerRange range) { m_selectedTextMarkerRange = range; } + AXTextMarkerRange selectedTextMarkerRange(); + void setSelectedTextMarkerRange(AXTextMarkerRange); AG: we should pass AXTextMarkerRange&& instead of by value. private: AXIsolatedTree(AXObjectCache&); AG: Add the macro WTF_GUARDED_BY_LOCK to the declaration: AXTextMarkerRange m_selectedTextMarkerRange;
Joshua Hoffman
Comment 4 2023-11-01 23:01:01 PDT
Created attachment 468452 [details] Patch
EWS
Comment 5 2023-11-02 13:01:31 PDT
Committed 270134@main (3e8a39cebd79): <https://commits.webkit.org/270134@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468452 [details].
Note You need to log in before you can comment on or make changes to this bug.