In a few places, we call AccessibilityUIElement::create() with a nullptr. This returns a Ref<> backed by a null pointer / object, which is confusing. We should probably return a null RefPtr instead. One example: Ref<AccessibilityUIElement> AccessibilityController::focusedElement() { auto page = InjectedBundle::singleton().page()->page(); auto root = static_cast<PlatformUIElement>(WKAccessibilityRootObject(page)); auto rootElement = AccessibilityUIElement::create(root); if (auto focusedElement = rootElement->focusedElement()) return *focusedElement; return AccessibilityUIElement::create(nullptr); }
<rdar://problem/85420387>
Created attachment 444279 [details] Patch
Created attachment 444287 [details] Patch
Created attachment 444288 [details] Patch
(In reply to Tyler Wilcock from comment #2) > Created attachment 444279 [details] > Patch --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp -Ref<AccessibilityUIElement> AccessibilityUIElement::create(PlatformUIElement uiElement) +RefPtr<AccessibilityUIElement> AccessibilityUIElement::create(PlatformUIElement uiElement) I think it is odd that a class create method can return null. Haven't seen examples of that in WebKit, but I can be convinced otherwise. Instead I think we shouldn't try to create an AccessibilityUIElement if the platform element is null. + if (!uiElement) + return { }; If this is the way to go, return nullptr here. --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h - static Ref<AccessibilityUIElement> create(PlatformUIElement); + static RefPtr<AccessibilityUIElement> create(PlatformUIElement); Again, create shouldn't return null IMHO.
Created attachment 444290 [details] Patch
(In reply to Andres Gonzalez from comment #5) > (In reply to Tyler Wilcock from comment #2) > > Created attachment 444279 [details] > > Patch > > --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp > +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp > > -Ref<AccessibilityUIElement> > AccessibilityUIElement::create(PlatformUIElement uiElement) > +RefPtr<AccessibilityUIElement> > AccessibilityUIElement::create(PlatformUIElement uiElement) > > I think it is odd that a class create method can return null. Haven't seen > examples of that in WebKit, but I can be convinced otherwise. Instead I > think we shouldn't try to create an AccessibilityUIElement if the platform > element is null. > > + if (!uiElement) > + return { }; > > If this is the way to go, return nullptr here. > > --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h > +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h > > - static Ref<AccessibilityUIElement> create(PlatformUIElement); > + static RefPtr<AccessibilityUIElement> create(PlatformUIElement); > > Again, create shouldn't return null IMHO. Yeah, I think that's reasonable. I checked, and you're right that every create method everywhere returns a Ref. I'll change it.
Created attachment 444304 [details] Patch
Created attachment 444310 [details] Patch
(In reply to Tyler Wilcock from comment #9) > Created attachment 444310 [details] > Patch --- a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp +++ a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp -Ref<AccessibilityUIElement> AccessibilityController::focusedElement() +RefPtr<AccessibilityUIElement> AccessibilityController::focusedElement() { notImplemented(); return AccessibilityUIElement::create(nullptr); we should return nullptr here for consistency.
Created attachment 444321 [details] Patch
Committed r285859 (244285@main): <https://commits.webkit.org/244285@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444321 [details].