[webkit-changes] [WebKit/WebKit] 3cc438: AX: 8 properties are unnecessarily cached for ever...

[Tyler Wilcock]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 3cc438654ebc758e803dfa01b3ab638ace726a96
      https://github.com/WebKit/WebKit/commit/3cc438654ebc758e803dfa01b3ab638ace726a96
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2025-03-28 (Fri, 28 Mar 2025)

  Changed paths:
    M Source/WebCore/accessibility/AXCoreObject.cpp
    M Source/WebCore/accessibility/AXCoreObject.h
    M Source/WebCore/accessibility/AXLogger.cpp
    M Source/WebCore/accessibility/AXObjectCache.cpp
    M Source/WebCore/accessibility/AccessibilityNodeObject.cpp
    M Source/WebCore/accessibility/AccessibilityNodeObject.h
    M Source/WebCore/accessibility/AccessibilityObject.cpp
    M Source/WebCore/accessibility/AccessibilityObject.h
    M Source/WebCore/accessibility/cocoa/AXCoreObjectCocoa.mm
    M Source/WebCore/accessibility/cocoa/AccessibilityObjectCocoa.mm
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
    M Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm
    M Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm

  Log Message:
  -----------
  AX: 8 properties are unnecessarily cached for every single AXIsolatedObject
https://bugs.webkit.org/show_bug.cgi?id=290585
rdar://148061952

Reviewed by Chris Fleizach.

Eight properties were unnecessarily cached for every isolated object, and are no longer cached except in the rare case
they are non-default.

AXProperty::ColorValue was always cached because our "check for default value" logic in AXIsolatedObject::setProperty
compared against Color(), which is transparent black, but AccessibilityNodeObject::colorValue returns Color::black
for anything that isn't a color input, and these are not equal. This is fixed by some special-case handling when
caching and returning AXProperty::ColorValue.

Furthermore, our is-default check for `Color`s is generally much smarter and caches far fewer values, e.g. for properties
like AXProperty::BackgroundColor and AXProperty::LinethroughColor. Prior to this commit, we compare to Color() without
converting to RGBA, which is a lossy conversion. This meant that the colors would be effectively the same as the default,
but still get cached. Now we eagerly convert to SRGBA before comparing colors.

AXProperty::{InvalidStatus, AutoCompleteValue, PopupValue, LiveRegionRelevant} all fell back to returning a non-empty
string in the default case when the corresponding attribute wasn't present, meaning we always cached something for these
properties. With this commit, we now only cache something when a value is explicitly set.

AXProperty::SpeechHint has been changed to AXProperty::SpeakAs. This propery was similar to the previous group - we always
computed a non-empty string, and thus always cached something.

AXProperty::StringValue used to cache null strings, but not empty ones, as an optimization. Prior to ENABLE(AX_THREAD_TEXT_APIS),
we needed this distinction to know whether we should fallback to using the cached AXProperty::AttributedString as the string value.
However, with ENABLE(AX_THREAD_TEXT_APIS), we now compute attributed strings on-the-fly, and thus don't need to avoid
considering null string values to be default, allowing us to avoid caching this property ~95% more often (based on data from gmail.com).

AXProperty::SortDirection always used to be cached because our is-default check looked for AccessibilitySortDirection::None,
but for anything that column header or row header, we would return AccessibilitySortDirection::Invalid. This commit addresses
the issue by making Invalid the first variant in the enum, meaning it will be the default, since we cast this enum to an
int before caching it.

This saves roughly ~218 mb of memory on http://html.spec.whatwg.org.

(567769 objects * 8 fewer properties per object * 48 bytes (sizeof(AXPropertyValueVariant)))

The savings are probably even better considering the significantly smarter Color default checks.

* Source/WebCore/accessibility/AXCoreObject.cpp:
(WebCore::AXCoreObject::autoCompleteValue const):
(WebCore::AXCoreObject::invalidStatus const):
(WebCore::AXCoreObject::popupValue const):
* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::AXCoreObject::liveRegionRelevant const):
* Source/WebCore/accessibility/AXLogger.cpp:
(WebCore::operator<<):
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::updateIsolatedTree):
* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::liveRegionRelevant const): Deleted.
* Source/WebCore/accessibility/AccessibilityNodeObject.h:
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::speakAs const):
(WebCore::AccessibilityObject::explicitAutoCompleteValue const):
(WebCore::AccessibilityObject::explicitInvalidStatus const):
(WebCore::AccessibilityObject::explicitPopupValue const):
(WebCore::AccessibilityObject::speakAsProperty const): Deleted.
(WebCore::AccessibilityObject::autoCompleteValue const): Deleted.
(WebCore::AccessibilityObject::invalidStatus const): Deleted.
(WebCore::AccessibilityObject::popupValue const): Deleted.
* Source/WebCore/accessibility/AccessibilityObject.h:
* Source/WebCore/accessibility/cocoa/AXCoreObjectCocoa.mm:
(WebCore::AXCoreObject::speechHint const):
* Source/WebCore/accessibility/cocoa/AccessibilityObjectCocoa.mm:
(WebCore::AccessibilityObject::speechHintAttributeValue const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeProperties):
(WebCore::AXIsolatedObject::setProperty):
(WebCore::AXIsolatedObject::colorValue const):
(WebCore::AXIsolatedObject::optionSetAttributeValue const):
(WebCore::AXIsolatedObject::stringValue const):
(WebCore::AXIsolatedObject::ancestorFlags const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::updateNodeProperties):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
* Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:
(WebCore::AXIsolatedObject::initializePlatformProperties):
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
(-[WebAccessibilityObjectWrapperBase baseAccessibilitySpeechHint]):

Canonical link: https://commits.webkit.org/292834@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications