- Revision
- 285859
- Author
- commit-qu...@webkit.org
- Date
- 2021-11-16 06:38:18 -0800 (Tue, 16 Nov 2021)
Log Message
AX: Stop returning AccessibilityUIElements backed by a null pointer
https://bugs.webkit.org/show_bug.cgi?id=233138
Patch by Tyler Wilcock <tyle...@apple.com> on 2021-11-16
Reviewed by Andres Gonzalez.
Prior to this patch, some callers of AccessibilityUIElement::create
passed null `id`s, resulting in a seemingly valid AccessibilityUIElement
that is actually backed by a null pointer. This makes writing tests harder,
since you can't rely on common operations like `!myElement` to do the right thing.
With this patch, we add a RELEASE_ASSERT to
AccessibilityUIElement::create requiring the passed pointer to be
valid. This patch also fixes up some callsites where we could (and did) pass null pointers.
* WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp:
(WTR::AccessibilityController::focusedElement):
* WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp:
(WTR::AccessibilityController::focusedElement):
* WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:
(WTR::AccessibilityController::focusedElement):
Return RefPtr instead of Ref, since it's possible that no element is
focused.
* WebKitTestRunner/InjectedBundle/AccessibilityController.h:
* WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp:
(WTR::AccessibilityUIElement::create):
Add RELEASE_ASSERT() to verify passed pointer is valid.
* WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
(WTR::AccessibilityUIElement::focusedElement const):
(WTR::AccessibilityUIElement::uiElementForSearchPredicate):
* WebKitTestRunner/InjectedBundle/mac/AccessibilityNotificationHandler.mm:
(-[AccessibilityNotificationHandler _notificationReceived:]):
* WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::uiElementAttributeValue const):
(WTR::AccessibilityUIElement::uiElementForSearchPredicate):
(WTR::AccessibilityUIElement::cellForColumnAndRow):
(WTR::AccessibilityUIElement::horizontalScrollbar const):
(WTR::AccessibilityUIElement::verticalScrollbar const):
Don't call AccessibilityUIElement::create with null pointers.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (285858 => 285859)
--- trunk/Tools/ChangeLog 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/ChangeLog 2021-11-16 14:38:18 UTC (rev 285859)
@@ -1,3 +1,46 @@
+2021-11-16 Tyler Wilcock <tyle...@apple.com>
+
+ AX: Stop returning AccessibilityUIElements backed by a null pointer
+ https://bugs.webkit.org/show_bug.cgi?id=233138
+
+ Reviewed by Andres Gonzalez.
+
+ Prior to this patch, some callers of AccessibilityUIElement::create
+ passed null `id`s, resulting in a seemingly valid AccessibilityUIElement
+ that is actually backed by a null pointer. This makes writing tests harder,
+ since you can't rely on common operations like `!myElement` to do the right thing.
+
+ With this patch, we add a RELEASE_ASSERT to
+ AccessibilityUIElement::create requiring the passed pointer to be
+ valid. This patch also fixes up some callsites where we could (and did) pass null pointers.
+
+ * WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp:
+ (WTR::AccessibilityController::focusedElement):
+ * WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp:
+ (WTR::AccessibilityController::focusedElement):
+ * WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:
+ (WTR::AccessibilityController::focusedElement):
+ Return RefPtr instead of Ref, since it's possible that no element is
+ focused.
+
+ * WebKitTestRunner/InjectedBundle/AccessibilityController.h:
+ * WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp:
+ (WTR::AccessibilityUIElement::create):
+ Add RELEASE_ASSERT() to verify passed pointer is valid.
+
+ * WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
+ (WTR::AccessibilityUIElement::focusedElement const):
+ (WTR::AccessibilityUIElement::uiElementForSearchPredicate):
+ * WebKitTestRunner/InjectedBundle/mac/AccessibilityNotificationHandler.mm:
+ (-[AccessibilityNotificationHandler _notificationReceived:]):
+ * WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
+ (WTR::AccessibilityUIElement::uiElementAttributeValue const):
+ (WTR::AccessibilityUIElement::uiElementForSearchPredicate):
+ (WTR::AccessibilityUIElement::cellForColumnAndRow):
+ (WTR::AccessibilityUIElement::horizontalScrollbar const):
+ (WTR::AccessibilityUIElement::verticalScrollbar const):
+ Don't call AccessibilityUIElement::create with null pointers.
+
2021-11-15 Michael Catanzaro <mcatanz...@gnome.org>
Make valgrind work properly without extra environment variables
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp 2021-11-16 14:38:18 UTC (rev 285859)
@@ -90,14 +90,12 @@
return AccessibilityUIElement::create(root);
}
-Ref<AccessibilityUIElement> AccessibilityController::focusedElement()
+RefPtr<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);
+ return rootElement->focusedElement();
}
void AccessibilityController::executeOnAXThreadAndWait(Function<void()>&& function)
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h 2021-11-16 14:38:18 UTC (rev 285859)
@@ -63,7 +63,7 @@
// Controller Methods - platform-independent implementations.
#if HAVE(ACCESSIBILITY)
Ref<AccessibilityUIElement> rootElement();
- Ref<AccessibilityUIElement> focusedElement();
+ RefPtr<AccessibilityUIElement> focusedElement();
#endif
RefPtr<AccessibilityUIElement> elementAtPoint(int x, int y);
RefPtr<AccessibilityUIElement> accessibleElementById(JSStringRef idAttribute);
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp 2021-11-16 14:38:18 UTC (rev 285859)
@@ -35,6 +35,7 @@
Ref<AccessibilityUIElement> AccessibilityUIElement::create(PlatformUIElement uiElement)
{
+ RELEASE_ASSERT(uiElement);
return adoptRef(*new AccessibilityUIElement(uiElement));
}
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp 2021-11-16 14:38:18 UTC (rev 285859)
@@ -112,7 +112,7 @@
return AccessibilityUIElement::create(static_cast<AtkObject*>(root));
}
-Ref<AccessibilityUIElement> AccessibilityController::focusedElement()
+RefPtr<AccessibilityUIElement> AccessibilityController::focusedElement()
{
WKBundlePageRef page = InjectedBundle::singleton().page()->page();
void* root = WKAccessibilityFocusedObject(page);
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm 2021-11-16 14:38:18 UTC (rev 285859)
@@ -635,7 +635,9 @@
RefPtr<AccessibilityUIElement> AccessibilityUIElement::focusedElement() const
{
- return AccessibilityUIElement::create([m_element accessibilityFocusedUIElement]);
+ if (id focusedUIElement = [m_element accessibilityFocusedUIElement])
+ return AccessibilityUIElement::create(focusedUIElement);
+ return nullptr;
}
bool AccessibilityUIElement::isFocused() const
@@ -765,14 +767,18 @@
RefPtr<AccessibilityUIElement> AccessibilityUIElement::uiElementForSearchPredicate(JSContextRef context, AccessibilityUIElement *startElement, bool isDirectionNext, JSValueRef searchKey, JSStringRef searchText, bool visibleOnly, bool immediateDescendantsOnly)
{
NSDictionary *parameterizedAttribute = searchPredicateParameterizedAttributeForSearchCriteria(context, startElement, isDirectionNext, 5, searchKey, searchText, visibleOnly, immediateDescendantsOnly);
- id value = [m_element accessibilityFindMatchingObjects:parameterizedAttribute];
- if (![value isKindOfClass:[NSArray class]])
+ id results = [m_element accessibilityFindMatchingObjects:parameterizedAttribute];
+ if (![results isKindOfClass:[NSArray class]])
return nullptr;
- for (id element in value) {
+
+ for (id element in results) {
if ([element isAccessibilityElement])
return AccessibilityUIElement::create(element);
}
- return AccessibilityUIElement::create([value firstObject]);
+
+ if (id firstResult = [results firstObject])
+ return AccessibilityUIElement::create(firstResult);
+ return nullptr;
}
JSRetainPtr<JSStringRef> AccessibilityUIElement::selectTextWithCriteria(JSContextRef, JSStringRef ambiguityResolution, JSValueRef searchStrings, JSStringRef replacementString, JSStringRef activity)
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityNotificationHandler.mm (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityNotificationHandler.mm 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityNotificationHandler.mm 2021-11-16 14:38:18 UTC (rev 285859)
@@ -127,7 +127,8 @@
} else {
// A global listener gets the element, notification name and userInfo.
JSValueRef arguments[3];
- arguments[0] = toJS(context, WTR::AccessibilityUIElement::create([notification object]).ptr());
+ id notificationObject = [notification object];
+ arguments[0] = toJS(context, notificationObject ? WTR::AccessibilityUIElement::create(notificationObject).ptr() : nullptr);
arguments[1] = notificationNameArgument;
arguments[2] = userInfoArgument;
JSObjectCallAsFunction(context, const_cast<JSObjectRef>(m_notificationFunctionCallback), 0, 3, arguments, 0);
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm 2021-11-16 14:38:18 UTC (rev 285859)
@@ -617,8 +617,9 @@
RefPtr<AccessibilityUIElement> AccessibilityUIElement::uiElementAttributeValue(JSStringRef attribute) const
{
- auto value = attributeValue([NSString stringWithJSStringRef:attribute]);
- return AccessibilityUIElement::create(value.get());
+ if (auto value = attributeValue([NSString stringWithJSStringRef:attribute]))
+ return AccessibilityUIElement::create(value.get());
+ return nullptr;
}
bool AccessibilityUIElement::boolAttributeValue(JSStringRef attribute)
@@ -1241,9 +1242,11 @@
{
BEGIN_AX_OBJC_EXCEPTIONS
NSDictionary *parameterizedAttribute = searchPredicateParameterizedAttributeForSearchCriteria(context, startElement, isDirectionNext, 1, searchKey, searchText, visibleOnly, immediateDescendantsOnly);
- id value = [m_element accessibilityAttributeValue:@"AXUIElementsForSearchPredicate" forParameter:parameterizedAttribute];
- if ([value isKindOfClass:[NSArray class]])
- return AccessibilityUIElement::create([value lastObject]);
+ id searchResults = [m_element accessibilityAttributeValue:@"AXUIElementsForSearchPredicate" forParameter:parameterizedAttribute];
+ if ([searchResults isKindOfClass:[NSArray class]]) {
+ if (id lastResult = [searchResults lastObject])
+ return AccessibilityUIElement::create(lastResult);
+ }
END_AX_OBJC_EXCEPTIONS
return nullptr;
@@ -1410,7 +1413,8 @@
{
NSArray *colRowArray = @[@(col), @(row)];
BEGIN_AX_OBJC_EXCEPTIONS
- return AccessibilityUIElement::create([m_element accessibilityAttributeValue:@"AXCellForColumnAndRow" forParameter:colRowArray]);
+ if (id cell = [m_element accessibilityAttributeValue:@"AXCellForColumnAndRow" forParameter:colRowArray])
+ return AccessibilityUIElement::create(cell);
END_AX_OBJC_EXCEPTIONS
return nullptr;
@@ -1422,7 +1426,8 @@
return nullptr;
BEGIN_AX_OBJC_EXCEPTIONS
- return AccessibilityUIElement::create(attributeValue(NSAccessibilityHorizontalScrollBarAttribute).get());
+ if (id scrollbar = attributeValue(NSAccessibilityHorizontalScrollBarAttribute).get())
+ return AccessibilityUIElement::create(scrollbar);
END_AX_OBJC_EXCEPTIONS
return nullptr;
@@ -1434,7 +1439,8 @@
return nullptr;
BEGIN_AX_OBJC_EXCEPTIONS
- return AccessibilityUIElement::create(attributeValue(NSAccessibilityVerticalScrollBarAttribute).get());
+ if (id scrollbar = attributeValue(NSAccessibilityVerticalScrollBarAttribute).get())
+ return AccessibilityUIElement::create(scrollbar);
END_AX_OBJC_EXCEPTIONS
return nullptr;
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp (285858 => 285859)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp 2021-11-16 13:40:51 UTC (rev 285858)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp 2021-11-16 14:38:18 UTC (rev 285859)
@@ -61,10 +61,10 @@
return AccessibilityUIElement::create(nullptr);
}
-Ref<AccessibilityUIElement> AccessibilityController::focusedElement()
+RefPtr<AccessibilityUIElement> AccessibilityController::focusedElement()
{
notImplemented();
- return AccessibilityUIElement::create(nullptr);
+ return nullptr;
}
bool AccessibilityController::addNotificationListener(JSValueRef)