Skip to content

Commit

Permalink
AX: Move attributedStringForNSRange off the main thread when possible.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=255789
<rdar://problem/108369924>

Reviewed by Chris Fleizach.

This patch avoids hitting the main thread for requests of the AttributedString for NSRanges contained in a text control. That is the most common case during text editing. To accomplish this, textMarkerRangeForNSRange needs to build a TextMarkerRange based on CharacterOffsets and not in VisiblePositions. In addition, the ranges used in caching the TextContent and the AttributedString also need to be based on CharacterOffsets.

* Source/WebCore/accessibility/AXTextMarker.h:
(WebCore::AXTextMarkerRange::operator bool const): Clean up.
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::textContent const):
* Source/WebCore/accessibility/cocoa/AccessibilityObjectCocoa.mm:
(WebCore::AccessibilityObject::textMarkerRangeForNSRange const):
* Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:
(WebCore::AXIsolatedObject::initializePlatformProperties):
(WebCore::AXIsolatedObject::textMarkerRangeForNSRange const):
(WebCore::AXIsolatedObject::attributedStringForTextMarkerRange const):
(WebCore::AXIsolatedObject::cachedAttributedStringForTextMarkerRange const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper attributedStringForNSRange:]):
(-[WebAccessibilityObjectWrapper rtfForNSRange:]):
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]):
(-[WebAccessibilityObjectWrapper doAXAttributedStringForRange:]): Renamed.
(-[WebAccessibilityObjectWrapper doAXRTFForRange:]): Renamed.

Canonical link: https://commits.webkit.org/263270@main
  • Loading branch information
AndresGonzalezApple committed Apr 22, 2023
1 parent 465af8b commit d910e37
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 34 deletions.
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AXTextMarker.h
Expand Up @@ -151,7 +151,7 @@ class AXTextMarkerRange {
AXTextMarkerRange(AXID treeID, AXID objectID, unsigned offset, unsigned length);
AXTextMarkerRange() = default;

operator bool() const { return !m_start.isNull() && !m_end.isNull(); }
operator bool() const { return m_start && m_end; }
operator VisiblePositionRange() const;
std::optional<SimpleRange> simpleRange() const;

Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/accessibility/AccessibilityObject.cpp
Expand Up @@ -2846,7 +2846,12 @@ std::optional<String> AccessibilityObject::textContent() const
if (!hasTextContent())
return std::nullopt;

if (auto range = simpleRange())
std::optional<SimpleRange> range;
if (isTextControl())
range = rangeForPlainTextRange({ 0, text().length() });
else
range = simpleRange();
if (range)
return stringForRange(*range);
return std::nullopt;
}
Expand Down
14 changes: 13 additions & 1 deletion Source/WebCore/accessibility/cocoa/AccessibilityObjectCocoa.mm
Expand Up @@ -217,7 +217,19 @@ static bool isDescriptiveText(AccessibilityTextSource textSource)
{
if (range.location == NSNotFound)
return { };
return { visiblePositionForIndex(range.location), visiblePositionForIndex(range.location + range.length) };

if (!isTextControl())
return { visiblePositionForIndex(range.location), visiblePositionForIndex(range.location + range.length) };

if (range.location + range.length > text().length())
return { };

if (auto* cache = axObjectCache()) {
auto start = cache->characterOffsetForIndex(range.location, this);
auto end = cache->characterOffsetForIndex(range.location + range.length, this);
return cache->rangeForUnorderedCharacterOffsets(start, end);
}
return { };
}

// NSAttributedString support.
Expand Down
Expand Up @@ -473,7 +473,6 @@ class AXIsolatedObject final : public AXCoreObject {
unsigned textLength() const override;
#if PLATFORM(COCOA)
RetainPtr<NSAttributedString> attributedStringForTextMarkerRange(AXTextMarkerRange&&, SpellCheck) const override;
NSAttributedString *cachedAttributedStringForTextMarkerRange(const AXTextMarkerRange&, SpellCheck) const;
#endif
AXObjectCache* axObjectCache() const override;
Element* actionElement() const override;
Expand Down
Expand Up @@ -41,7 +41,12 @@

RetainPtr<NSAttributedString> attributedText;
if (object->hasAttributedText()) {
if (auto range = object->simpleRange()) {
std::optional<SimpleRange> range;
if (isTextControl())
range = rangeForPlainTextRange({ 0, object->text().length() });
else
range = object->simpleRange();
if (range) {
if ((attributedText = object->attributedStringForRange(*range, SpellCheck::Yes)))
setProperty(AXPropertyName::AttributedText, attributedText);
}
Expand Down Expand Up @@ -163,29 +168,25 @@

RetainPtr<NSAttributedString> AXIsolatedObject::attributedStringForTextMarkerRange(AXTextMarkerRange&& markerRange, SpellCheck spellCheck) const
{
if (NSAttributedString *cachedString = cachedAttributedStringForTextMarkerRange(markerRange, spellCheck))
return cachedString;

return Accessibility::retrieveValueFromMainThread<RetainPtr<NSAttributedString>>([markerRange = WTFMove(markerRange), &spellCheck, this] () mutable -> RetainPtr<NSAttributedString> {
if (RefPtr axObject = associatedAXObject())
return axObject->attributedStringForTextMarkerRange(WTFMove(markerRange), spellCheck);
return { };
});
}
if (!markerRange)
return nil;

NSAttributedString *AXIsolatedObject::cachedAttributedStringForTextMarkerRange(const AXTextMarkerRange& markerRange, SpellCheck spellCheck) const
{
// At the moment we are only handling ranges that are contained in a single object, and for which we cached the AttributeString.
// FIXME: Extend to cases where the range expands multiple objects.

auto attributedText = propertyValue<RetainPtr<NSAttributedString>>(AXPropertyName::AttributedText);
if (!attributedText) {
return Accessibility::retrieveValueFromMainThread<RetainPtr<NSAttributedString>>([markerRange = WTFMove(markerRange), &spellCheck, this] () mutable -> RetainPtr<NSAttributedString> {
if (RefPtr axObject = associatedAXObject())
return axObject->attributedStringForTextMarkerRange(WTFMove(markerRange), spellCheck);
return { };
});
}

auto nsRange = markerRange.nsRange();
if (!nsRange)
return nil;

auto attributedText = propertyValue<RetainPtr<NSAttributedString>>(AXPropertyName::AttributedText);
if (!attributedText)
return nil;

if (!attributedStringContainsRange(attributedText.get(), *nsRange))
return nil;

Expand Down
Expand Up @@ -2804,16 +2804,12 @@ - (NSString*)accessibilityActionDescription:(NSString*)action

// The CFAttributedStringType representation of the text associated with this accessibility
// object that is specified by the given range.
- (NSAttributedString *)doAXAttributedStringForRange:(NSRange)range
- (NSAttributedString *)attributedStringForNSRange:(const NSRange&)range
{
return Accessibility::retrieveAutoreleasedValueFromMainThread<NSAttributedString *>([&range, protectedSelf = retainPtr(self)] () -> RetainPtr<NSAttributedString> {
auto* backingObject = protectedSelf.get().axBackingObject;
if (!backingObject)
return nil;

auto webRange = backingObject->rangeForPlainTextRange(range);
return [protectedSelf attributedStringForTextMarkerRange:textMarkerRangeFromRange(backingObject->axObjectCache(), webRange) spellCheck:AXCoreObject::SpellCheck::Yes];
});
auto* backingObject = self.axBackingObject;
if (!backingObject)
return nil;
return backingObject->attributedStringForTextMarkerRange(backingObject->textMarkerRangeForNSRange(range), AXCoreObject::SpellCheck::Yes).autorelease();
}

- (NSInteger)_indexForTextMarker:(AXTextMarkerRef)markerRef
Expand Down Expand Up @@ -2857,9 +2853,9 @@ - (AXTextMarkerRef)_textMarkerForIndex:(NSInteger)textIndex

// The RTF representation of the text associated with this accessibility object that is
// specified by the given range.
- (NSData *)doAXRTFForRange:(NSRange)range
- (NSData *)rtfForNSRange:(const NSRange&)range
{
NSAttributedString *attrString = [self doAXAttributedStringForRange:range];
NSAttributedString *attrString = [self attributedStringForNSRange:range];
return [attrString RTFFromRange:NSMakeRange(0, attrString.length) documentAttributes:@{ }];
}

Expand Down Expand Up @@ -3395,6 +3391,7 @@ - (id)accessibilityAttributeValue:(NSString*)attribute forParameter:(id)paramete
auto* cache = backingObject->axObjectCache();
if (!cache)
return String();

auto start = cache->characterOffsetForIndex(range.location, backingObject);
auto end = cache->characterOffsetForIndex(range.location + range.length, backingObject);
auto range = cache->rangeForUnorderedCharacterOffsets(start, end);
Expand Down Expand Up @@ -3630,11 +3627,11 @@ - (id)accessibilityAttributeValue:(NSString*)attribute forParameter:(id)paramete
return [NSValue valueWithRect:rect];
}

if ([attribute isEqualToString: (NSString*)kAXRTFForRangeParameterizedAttribute])
return rangeSet ? [self doAXRTFForRange:range] : nil;
if ([attribute isEqualToString:(NSString *)kAXRTFForRangeParameterizedAttribute])
return rangeSet ? [self rtfForNSRange:range] : nil;

if ([attribute isEqualToString:(NSString *)kAXAttributedStringForRangeParameterizedAttribute])
return rangeSet ? [self doAXAttributedStringForRange:range] : nil;
return rangeSet ? [self attributedStringForNSRange:range] : nil;

if ([attribute isEqualToString: (NSString*)kAXStyleRangeForIndexParameterizedAttribute]) {
PlainTextRange textRange = backingObject->doAXStyleRangeForIndex([number intValue]);
Expand Down

0 comments on commit d910e37

Please sign in to comment.