Bug 236115 - Cache some expensive AXIsolatedObject properties lazily.
Summary: Cache some expensive AXIsolatedObject properties lazily.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-03 18:26 PST by Andres Gonzalez
Modified: 2022-02-04 11:21 PST (History)
10 users (show)

See Also:


Attachments
Patch (9.12 KB, patch)
2022-02-03 18:45 PST, Andres Gonzalez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gonzalez 2022-02-03 18:26:41 PST
Cache some expensive AXIsolatedObject properties lazily.
Comment 1 Radar WebKit Bug Importer 2022-02-03 18:26:54 PST
<rdar://problem/88467667>
Comment 2 Andres Gonzalez 2022-02-03 18:45:59 PST
Created attachment 450850 [details]
Patch
Comment 3 chris fleizach 2022-02-03 19:59:06 PST
Comment on attachment 450850 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450850&action=review

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:978
> +String AXIsolatedObject::getOrRetrieveStringPropertyValue(AXPropertyName propertyName)

Get and retrieve seem like synonyms here so you could probably drop both and just call it stringPropertyValue()

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1009
> +        setProperty(propertyName, value.isolatedCopy());

should we cache if the string is empty
Comment 4 Andres Gonzalez 2022-02-04 05:16:54 PST
(In reply to chris fleizach from comment #3)
> Comment on attachment 450850 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450850&action=review
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:978
> > +String AXIsolatedObject::getOrRetrieveStringPropertyValue(AXPropertyName propertyName)
> 
> Get and retrieve seem like synonyms here so you could probably drop both and
> just call it stringPropertyValue()

The idea is to differentiate it from stringAttributeValue that I intend to rename stringPropertyValue. The getOrRetrieve makes explicit that the value is either gotten from the cache or retrieved from the main thread. The reason I'd like to rename stringAttributeValue -> stringPropertyValue is the confusing overload of "attribute" which is used for HTML element attributes, platform API methods, and AXCoreObject methods.

> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1009
> > +        setProperty(propertyName, value.isolatedCopy());
> 
> should we cache if the string is empty

I think so since that the behavior of the live object as well.
Comment 5 chris fleizach 2022-02-04 07:25:40 PST
Comment on attachment 450850 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450850&action=review

>>> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:978
>>> +String AXIsolatedObject::getOrRetrieveStringPropertyValue(AXPropertyName propertyName)
>> 
>> Get and retrieve seem like synonyms here so you could probably drop both and just call it stringPropertyValue()
> 
> The idea is to differentiate it from stringAttributeValue that I intend to rename stringPropertyValue. The getOrRetrieve makes explicit that the value is either gotten from the cache or retrieved from the main thread. The reason I'd like to rename stringAttributeValue -> stringPropertyValue is the confusing overload of "attribute" which is used for HTML element attributes, platform API methods, and AXCoreObject methods.

In this case maybe cachedStringPropertyValue would be good
Comment 6 EWS 2022-02-04 11:20:59 PST
Committed r289131 (246828@main): <https://commits.webkit.org/246828@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450850 [details].