[webkit-changes] [WebKit/WebKit] 291a72: AX: A new AccessibilityMenuListPopup is created an...

[Tyler Wilcock]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 291a726050b82ba5edb9f41bf4137ebbeadad477
      https://github.com/WebKit/WebKit/commit/291a726050b82ba5edb9f41bf4137ebbeadad477
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2024-11-22 (Fri, 22 Nov 2024)

  Changed paths:
    M Source/WebCore/SaferCPPExpectations/UncountedLocalVarsCheckerExpectations
    M Source/WebCore/accessibility/AXObjectCache.cpp
    M Source/WebCore/accessibility/AccessibilityMenuList.cpp
    M Source/WebCore/accessibility/AccessibilityMenuList.h
    M Source/WebCore/accessibility/AccessibilitySpinButton.h

  Log Message:
  -----------
  AX: A new AccessibilityMenuListPopup is created and never cleaned up for every clearChildren-addChildren cycle of AccessibilityMenuList, causing a memory leak
https://bugs.webkit.org/show_bug.cgi?id=283468
rdar://140323875

Reviewed by Chris Fleizach.

Prior to this commit, every time an AccessibilityMenuList calls clearChildren() and addChildren(), we would create
a brand new AccessibilityMenuListPopup object with an associated object wrapper, without cleaning up the popup
from the last cycle, leaking it.

With this commit, we now create the AccessibilityMenuListPopup up-front in the AccessibilityMenuList constructor, and
maintain it between clearChildren-addChildren cycles. This is also better because it keeps the popup wrapper more stable
for ATs, i.e. we are no longer at risk of sending a notification with a popup, then calling clearChildren,
effectively detaching the popup the AT has a reference to, making it useless to the AT if it tries to respond to the notification.

This commit does not fully fix the memory leak, just greatly reduces it, leaking one object total instead of one object
per clearChildren-addChildren cycle. Because nothing calls AXObjectCache::remove on the new AccessibilityMenuList::m_popup
field, it gets leaked. I will be addressing this in a later commit once I think of a good pattern to solve this problem
holistically, since several classes have this bug in slightly different ways.

* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::createObjectFromRenderer):
* Source/WebCore/accessibility/AccessibilityMenuList.cpp:
(WebCore::AccessibilityMenuList::AccessibilityMenuList):
(WebCore::AccessibilityMenuList::create):
(WebCore::AccessibilityMenuList::press):
(WebCore::AccessibilityMenuList::addChildren):
(WebCore::AccessibilityMenuList::canSetFocusAttribute const):
(WebCore::AccessibilityMenuList::didUpdateActiveOption):
* Source/WebCore/accessibility/AccessibilityMenuList.h:
* Source/WebCore/accessibility/AccessibilitySpinButton.h:

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



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