Isolated object implementation of parameterized attribute SelectTextWithCriteria.
Created attachment 385625 [details] Patch
Comment on attachment 385625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385625&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:-3577 > - ASSERT(roleValue() == AccessibilityRole::TabList); worried that this might populate tabChildren for elements not expected to have any... maybe we can change this to if (role != TabList) return > Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.cpp:696 > +Vector<RefPtr<Range>> AXIsolatedObject::findTextRanges(AccessibilitySearchTextCriteria const& criteria) const what's the implication of returning a Range from the main thread to a secondary thread? > Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:101 > m_semaphore.signal(); can you rename execute to something that is more meaningful in explaining that it will dispatch to the main thread > Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:104 > + CFRunLoopRunInMode(kCFRunLoopDefaultMode, .25, false); put a comment why we need to spin the run loop
Created attachment 385642 [details] Patch
(In reply to chris fleizach from comment #2) > Comment on attachment 385625 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385625&action=review > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:-3577 > > - ASSERT(roleValue() == AccessibilityRole::TabList); > > worried that this might populate tabChildren for elements not expected to > have any... maybe we can change this to > > if (role != TabList) > return Done. > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.cpp:696 > > +Vector<RefPtr<Range>> AXIsolatedObject::findTextRanges(AccessibilitySearchTextCriteria const& criteria) const > > what's the implication of returning a Range from the main thread to a > secondary thread? This function is only used in the wrapper and will be always be executed inside a block dispatched to the main thread. You are right, it won't work if we return this value outside the main thread, but that is not happening. > > > Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:101 > > m_semaphore.signal(); > > can you rename execute to something that is more meaningful in explaining > that it will dispatch to the main thread Renamed it executeOnAXThreadIfPossible. > > > Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:104 > > + CFRunLoopRunInMode(kCFRunLoopDefaultMode, .25, false); > > put a comment why we need to spin the run loop Done.
Created attachment 385766 [details] Patch
Comment on attachment 385766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385766&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:50 > + AXObjectCache* axObjectCache() const { return m_axObjectCache; } feel like we should assert main thread in this getter
(In reply to chris fleizach from comment #6) > Comment on attachment 385766 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385766&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:50 > > + AXObjectCache* axObjectCache() const { return m_axObjectCache; } > > feel like we should assert main thread in this getter Will do. Thanks.
Comment on attachment 385766 [details] Patch Clearing flags on attachment: 385766 Committed r253565: <https://trac.webkit.org/changeset/253565>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57976699>