Bug 256896

<rdar://problem/109462537>

Created attachment 466386 [details] Patch

Comment on attachment 466386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=466386&action=review > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1217 > + // FIXME: Remove the following methods from the AXCoreObject intterface and instead use methods such as axScrollView() if needed. Typo: intterface vs. interface > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1488 > - return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> { > - auto* backingObject = protectedSelf.get().axBackingObject; > - if (!backingObject) > - return nil; > - > - if (auto* fv = backingObject->documentFrameView()) > - return [fv->platformWidget() window]; > - > - return nil; > - }); > + RefPtr axScrollView = self.axBackingObject->axScrollView(); > + return axScrollView ? [axScrollView->platformWidget() window] : nil; So the lines above this call self.remoteAccessibilityParentObject which is defined as: - (id)remoteAccessibilityParentObject { auto* backingObject = self.axBackingObject; return backingObject ? backingObject->remoteParentObject() : nil; } And in turn, AXIsolatedObjectMac::remoteParentObject is defined as: RemoteAXObjectRef AXIsolatedObject::remoteParentObject() const { auto* scrollView = Accessibility::findAncestor<AXCoreObject>(*this, true, [] (const AXCoreObject& object) { return object.isScrollView(); }); return is<AXIsolatedObject>(scrollView) ? downcast<AXIsolatedObject>(scrollView)->m_remoteParent.get() : nil; } So when we run your new self.axBackingObject->axScrollView(), we will have already iterated for and failed to find a scroll view. So your new codepath seems redundant from what I can tell. Do you read that the same way?

Created attachment 466422 [details] Patch

(In reply to Tyler Wilcock from comment #3) > Comment on attachment 466386 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=466386&action=review > > > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1217 > > + // FIXME: Remove the following methods from the AXCoreObject intterface and instead use methods such as axScrollView() if needed. > > Typo: intterface vs. interface Fixed. > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1488 > > - return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> { > > - auto* backingObject = protectedSelf.get().axBackingObject; > > - if (!backingObject) > > - return nil; > > - > > - if (auto* fv = backingObject->documentFrameView()) > > - return [fv->platformWidget() window]; > > - > > - return nil; > > - }); > > + RefPtr axScrollView = self.axBackingObject->axScrollView(); > > + return axScrollView ? [axScrollView->platformWidget() window] : nil; > > So the lines above this call self.remoteAccessibilityParentObject which is > defined as: > > - (id)remoteAccessibilityParentObject > { > auto* backingObject = self.axBackingObject; > return backingObject ? backingObject->remoteParentObject() : nil; > } > > And in turn, AXIsolatedObjectMac::remoteParentObject is defined as: > > RemoteAXObjectRef AXIsolatedObject::remoteParentObject() const > { > auto* scrollView = Accessibility::findAncestor<AXCoreObject>(*this, > true, [] (const AXCoreObject& object) { > return object.isScrollView(); > }); > return is<AXIsolatedObject>(scrollView) ? > downcast<AXIsolatedObject>(scrollView)->m_remoteParent.get() : nil; > } > > So when we run your new self.axBackingObject->axScrollView(), we will have > already iterated for and failed to find a scroll view. So your new codepath > seems redundant from what I can tell. Do you read that the same way? No, self.remoteAccessibilityParentObject returns false not because it doesn't find the ScrollView, but because m_remoteParent is null in return is<AXIsolatedObject>(scrollView) ? downcast<AXIsolatedObject>(scrollView)->m_remoteParent.get() : nil; and thus we were hitting the main thread. You can have a ScrollView that has no remote parent by still must have a platform widget.

Committed 264266@main (cb345f2eccf0): <https://commits.webkit.org/264266@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 466422 [details].