[webkit-changes] [WebKit/WebKit] c80839: AX: AXIsolatedTree and some objects within are not...

Tyler Wilcock noreply at github.com
Mon Sep 12 19:05:07 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: c808394fb05e57b8d7deb36af8bf5b59f8a34b70
      https://github.com/WebKit/WebKit/commit/c808394fb05e57b8d7deb36af8bf5b59f8a34b70
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2022-09-12 (Mon, 12 Sep 2022)

  Changed paths:
    A LayoutTests/accessibility/ax-object-destroyed-on-reload-expected.txt
    A LayoutTests/accessibility/ax-object-destroyed-on-reload.html
    M LayoutTests/platform/glib/TestExpectations
    M LayoutTests/platform/ios/TestExpectations
    A LayoutTests/platform/ios/accessibility/ax-object-destroyed-on-reload-expected.txt
    M LayoutTests/platform/mac-wk1/TestExpectations
    M LayoutTests/platform/win/TestExpectations
    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 Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp
    M Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h
    M Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp
    M Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
    M Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityController.idl
    M Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm

  Log Message:
  -----------
  AX: AXIsolatedTree and some objects within are not cleaned up upon page reload or navigation
https://bugs.webkit.org/show_bug.cgi?id=244941
rdar://99574478

Reviewed by Chris Fleizach and Andres Gonzalez.

Prior to this patch, we never seem to properly clean up any `AXIsolatedTree`s
and the `AXIsolatedObject`s within when the page reloaded or when a navigation
happened.

This is because not all objects were cleaned up from the m_readerThreadMap. And
because each object (before this patch) had a RefPtr<AXIsolatedTree>, this meant
the tree never got destroyed either.

With this patch, each AXIsolatedObject now holds only a
WeakPtr<AXIsolatedTree> to the tree it belongs to. This greatly
simplifies the lifetime of each tree, which is now entirely governed by
the tree's representation in treeIDCache() and treePageCache().

This patch also changes how we clean up the objects from a tree slated
for destruction. Prior to this patch, AXIsolatedTree::clear() used to do this:

m_pendingSubtreeRemovals.append(m_rootNode->objectID());

This should work, but seems to miss objects. With this patch, we take a
more simple approach:

  1. In ~AXObjectCache, queue the associated AXIsolatedTree up for
     destruction on the secondary thread
  2. In AXIsolatedTree::applyPendingChanges, the tree now detects it
     must self-destruct, and detaches every object in its
     m_readerThreadMap.

This patch also changes AccessibilityUIElement::m_element to be a weak
pointer rather than a strong pointer, more accurately representing the
behavior of actual AX clients (which obviously can't hold strong
references to objects inside WebKit).

* LayoutTests/accessibility/ax-object-destroyed-on-reload-expected.txt: Added.
* LayoutTests/accessibility/ax-object-destroyed-on-reload.html: Added.
* LayoutTests/platform/glib/TestExpectations: Skip new test.
* LayoutTests/platform/ios/TestExpectations: Enable new test.
* LayoutTests/platform/ios/accessibility/ax-object-destroyed-on-reload-expected.txt: Added.
* LayoutTests/platform/mac-wk1/TestExpectations: Skip new test.
* LayoutTests/platform/win/TestExpectations: Skip new test.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::children):
(WebCore::AXIsolatedObject::updateBackingStore):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::queueForDestruction):
(WebCore::AXIsolatedTree::removeTreeForPageID):
(WebCore::AXIsolatedTree::focusedNode):
(WebCore::AXIsolatedTree::applyPendingChanges):
(WebCore::AXIsolatedTree::clear): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::WTF_GUARDED_BY_LOCK):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.cpp:
(WTR::AccessibilityController::setRetainedElement):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h:
(WTR::AccessibilityController::retainedElement):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp:
(WTR::AccessibilityUIElement::isValid const):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h:
(WTR::AccessibilityUIElement::platformUIElement):
* Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityController.idl:
* Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::attributeValue const):
(WTR::AccessibilityUIElement::allAttributes):
(WTR::AccessibilityUIElement::setBoolAttributeValue):
(WTR::AccessibilityUIElement::setValue):
(WTR::AccessibilityUIElement::isAttributeSupported):
(WTR::AccessibilityUIElement::setSelectedTextRange):
(WTR::AccessibilityUIElement::setSelectedTextMarkerRange):
(WTR::AccessibilityUIElement::setSelectedChild const):
(WTR::AccessibilityUIElement::setSelectedChildAtIndex const):
(WTR::AccessibilityUIElement::removeSelectionAtIndex const):
(WTR::AccessibilityUIElement::takeFocus):
(WTR::AccessibilityUIElement::resetSelectedTextMarkerRange):

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




More information about the webkit-changes mailing list