Skip to content
Permalink
Browse files
Calls to AXCoreObject::widget() have to be dispatch to the main threa…
…d in isolated tree mode.

https://bugs.webkit.org/show_bug.cgi?id=231766
<rdar://problem/84271039>

Reviewed by Chris Fleizach.

This patch fixes several dozens of tests that were crashing or timing out in isolated tree mode.

The WebAccessibilityObjectWrapper was calling the widget() method on the
AX thread, which creates problems because this object is not thread
safe and it is created on the main thread. To avoid this problem, we
have to dispatch those calls to the main thread. Since widget() is
called very often now, for every request of the children of a leaf node,
added AXCoreObject::isWidget() to be able to check for it without having
to return the Widget object. The IsWidget property is cached in the
AXIsolatedObject, hence limiting the number of times we have to hit the
main thread.

* accessibility/AccessibilityNodeObject.cpp:
Removed include header that is not used in this cpp.

* accessibility/AccessibilityObject.h:
* accessibility/AccessibilityObjectInterface.h:
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::isWidget const):
(WebCore::AccessibilityRenderObject::widget const):
* accessibility/AccessibilityRenderObject.h:
* accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeAttributeData):
(WebCore::AXIsolatedObject::widget const):
* accessibility/isolatedtree/AXIsolatedObject.h:
* accessibility/isolatedtree/AXIsolatedTree.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper renderWidgetChildren]):
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
(isMatchingPlugin):


Canonical link: https://commits.webkit.org/243074@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284266 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
AndresGonzalezApple committed Oct 15, 2021
1 parent 572b2e0 commit e501d056922fe73ed2ab3d5639c5a8ef2f58c391
Showing 10 changed files with 92 additions and 27 deletions.
@@ -1,3 +1,42 @@
2021-10-15 Andres Gonzalez <andresg_22@apple.com>

Calls to AXCoreObject::widget() have to be dispatch to the main thread in isolated tree mode.
https://bugs.webkit.org/show_bug.cgi?id=231766
<rdar://problem/84271039>

Reviewed by Chris Fleizach.

This patch fixes several dozens of tests that were crashing or timing out in isolated tree mode.

The WebAccessibilityObjectWrapper was calling the widget() method on the
AX thread, which creates problems because this object is not thread
safe and it is created on the main thread. To avoid this problem, we
have to dispatch those calls to the main thread. Since widget() is
called very often now, for every request of the children of a leaf node,
added AXCoreObject::isWidget() to be able to check for it without having
to return the Widget object. The IsWidget property is cached in the
AXIsolatedObject, hence limiting the number of times we have to hit the
main thread.

* accessibility/AccessibilityNodeObject.cpp:
Removed include header that is not used in this cpp.

* accessibility/AccessibilityObject.h:
* accessibility/AccessibilityObjectInterface.h:
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::isWidget const):
(WebCore::AccessibilityRenderObject::widget const):
* accessibility/AccessibilityRenderObject.h:
* accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeAttributeData):
(WebCore::AXIsolatedObject::widget const):
* accessibility/isolatedtree/AXIsolatedObject.h:
* accessibility/isolatedtree/AXIsolatedTree.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper renderWidgetChildren]):
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
(isMatchingPlugin):

2021-10-15 BJ Burg <bburg@apple.com>

[Cocoa] Web Inspector: handle Promise objects returned from evaluateScriptInExtensionTab
@@ -74,7 +74,6 @@
#include "TextControlInnerElements.h"
#include "UserGestureIndicator.h"
#include "VisibleUnits.h"
#include "Widget.h"
#include <wtf/StdLibExtras.h>
#include <wtf/text/StringBuilder.h>
#include <wtf/unicode/CharacterNames.h>
@@ -442,13 +442,16 @@ class AccessibilityObject : public AXCoreObject {
String selectedText() const override { return String(); }
String accessKey() const override { return nullAtom(); }
String actionVerb() const override;

bool isWidget() const override { return false; }
Widget* widget() const override { return nullptr; }
PlatformWidget platformWidget() const override { return nullptr; }
Widget* widgetForAttachmentView() const override { return nullptr; }

#if PLATFORM(COCOA)
RemoteAXObjectRef remoteParentObject() const override;
FloatRect convertRectToPlatformSpace(const FloatRect&, AccessibilityConversionSpace) const override;
#endif
Widget* widgetForAttachmentView() const override { return nullptr; }
Page* page() const override;
Document* document() const override;
FrameView* documentFrameView() const override;
@@ -85,7 +85,6 @@ class Path;
class QualifiedName;
class RenderObject;
class ScrollView;
class Widget;

struct AccessibilityText;
struct ScrollRectToVisibleOptions;
@@ -1187,13 +1186,17 @@ class AXCoreObject : public ThreadSafeRefCounted<AXCoreObject> {
virtual String selectedText() const = 0;
virtual String accessKey() const = 0;
virtual String actionVerb() const = 0;

// Widget support.
virtual bool isWidget() const = 0;
virtual Widget* widget() const = 0;
virtual PlatformWidget platformWidget() const = 0;
virtual Widget* widgetForAttachmentView() const = 0;

#if PLATFORM(COCOA)
virtual RemoteAXObjectRef remoteParentObject() const = 0;
virtual FloatRect convertRectToPlatformSpace(const FloatRect&, AccessibilityConversionSpace) const = 0;
#endif
virtual Widget* widgetForAttachmentView() const = 0;
virtual Page* page() const = 0;
virtual Document* document() const = 0;
virtual FrameView* documentFrameView() const = 0;
@@ -2006,11 +2006,14 @@ Document* AccessibilityRenderObject::document() const
return &m_renderer->document();
}

bool AccessibilityRenderObject::isWidget() const
{
return widget();
}

Widget* AccessibilityRenderObject::widget() const
{
if (!m_renderer || !is<RenderWidget>(*m_renderer))
return nullptr;
return downcast<RenderWidget>(*m_renderer).widget();
return is<RenderWidget>(m_renderer.get()) ? downcast<RenderWidget>(*m_renderer).widget() : nullptr;
}

AccessibilityObject* AccessibilityRenderObject::accessibilityParentForImageMap(HTMLMapElement* map) const
@@ -50,8 +50,7 @@ class Node;
class RenderTextControl;
class RenderView;
class VisibleSelection;
class Widget;


class AccessibilityRenderObject : public AccessibilityNodeObject, public CanMakeWeakPtr<AccessibilityRenderObject> {
public:
static Ref<AccessibilityRenderObject> create(RenderObject*);
@@ -136,6 +135,8 @@ class AccessibilityRenderObject : public AccessibilityNodeObject, public CanMake
String selectedText() const override;
String accessKey() const override;
String actionVerb() const override;

bool isWidget() const override;
Widget* widget() const override;
Widget* widgetForAttachmentView() const override;
AccessibilityChildrenVector documentLinks() override;
@@ -418,6 +418,9 @@ void AXIsolatedObject::initializeAttributeData(AXCoreObject& object, bool isRoot
setObjectVectorProperty(AXPropertyName::DocumentLinks, object.documentLinks());
}

if (object.isWidget())
setProperty(AXPropertyName::IsWidget, true);

initializePlatformProperties(object, isRoot);
}

@@ -2032,9 +2035,8 @@ TextIteratorBehaviors AXIsolatedObject::textIteratorBehaviorForTextRange() const

Widget* AXIsolatedObject::widget() const
{
if (auto* object = associatedAXObject())
return object->widget();
return nullptr;
auto* object = associatedAXObject();
return object ? object->widget() : nullptr;
}

PlatformWidget AXIsolatedObject::platformWidget() const
@@ -584,15 +584,17 @@ class AXIsolatedObject final : public AXCoreObject {
Path elementPath() const override { return pathAttributeValue(AXPropertyName::Path); };
bool supportsPath() const override { return boolAttributeValue(AXPropertyName::SupportsPath); }
TextIteratorBehaviors textIteratorBehaviorForTextRange() const override;

bool isWidget() const override { return boolAttributeValue(AXPropertyName::IsWidget); }
Widget* widget() const override;
PlatformWidget platformWidget() const override;

Widget* widgetForAttachmentView() const override;

HashMap<String, AXEditingStyleValueVariant> resolvedEditingStyles() const override;
#if PLATFORM(COCOA)
RemoteAXObjectRef remoteParentObject() const override;
FloatRect convertRectToPlatformSpace(const FloatRect&, AccessibilityConversionSpace) const override;
#endif
Widget* widgetForAttachmentView() const override;
Page* page() const override;
Document* document() const override;
FrameView* documentFrameView() const override;
@@ -228,6 +228,7 @@ enum class AXPropertyName : uint16_t {
IsValueAutofillAvailable,
IsVisible,
IsVisited,
IsWidget,
KeyShortcutsValue,
Language,
LayoutCount,
@@ -1660,15 +1660,23 @@ - (NSArray*)accessibilityAttributeNames
return objectAttributes;
}

- (NSArray*)renderWidgetChildren
- (NSArray *)renderWidgetChildren
{
auto* backingObject = self.axBackingObject;
if (!backingObject)
if (!backingObject || !backingObject->isWidget())
return nil;

auto* widget = backingObject->widget();
if (widget && widget->accessibilityObject())
return @[widget->accessibilityObject()];
id child = Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
auto* backingObject = protectedSelf.get().axBackingObject;
if (!backingObject)
return nil;

auto* widget = backingObject->widget();
return widget ? widget->accessibilityObject() : nil;
});

if (child)
return @[child];

ALLOW_DEPRECATED_DECLARATIONS_BEGIN
return [backingObject->platformWidget() accessibilityAttributeValue:NSAccessibilityChildrenAttribute];
@@ -2168,10 +2176,9 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName
return parent->wrapper();
}

if ([attributeName isEqualToString: NSAccessibilityChildrenAttribute] || [attributeName isEqualToString: NSAccessibilityChildrenInNavigationOrderAttribute]) {
if ([attributeName isEqualToString:NSAccessibilityChildrenAttribute] || [attributeName isEqualToString:NSAccessibilityChildrenInNavigationOrderAttribute]) {
if (!self.childrenVectorSize) {
NSArray* children = [self renderWidgetChildren];
if (children != nil)
if (NSArray *children = [self renderWidgetChildren])
return children;
}

@@ -3654,14 +3661,19 @@ - (AXTextMarkerRef)textMarkerForTextMarker:(AXTextMarkerRef)textMarker atUnit:(T
});
}

static BOOL isMatchingPlugin(AXCoreObject* axObject, const AccessibilitySearchCriteria& criteria)
static bool isMatchingPlugin(AXCoreObject* axObject, const AccessibilitySearchCriteria& criteria)
{
auto* widget = axObject->widget();
if (!is<PluginViewBase>(widget))
if (!axObject->isWidget())
return false;

return criteria.searchKeys.contains(AccessibilitySearchKey::AnyType)
&& (!criteria.visibleOnly || downcast<PluginViewBase>(widget)->isVisible());
return Accessibility::retrieveValueFromMainThread<bool>([&axObject, &criteria] () -> bool {
auto* widget = axObject->widget();
if (!is<PluginViewBase>(widget))
return false;

return criteria.searchKeys.contains(AccessibilitySearchKey::AnyType)
&& (!criteria.visibleOnly || downcast<PluginViewBase>(widget)->isVisible());
});
}

ALLOW_DEPRECATED_IMPLEMENTATIONS_BEGIN

0 comments on commit e501d05

Please sign in to comment.