[webkit-changes] [WebKit/WebKit] 71ce3f: REGRESSION(281440 at main): Web content can become in...

[Tyler Wilcock]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 71ce3f1d011ad76b976f01882cf2a81129815ff9
      https://github.com/WebKit/WebKit/commit/71ce3f1d011ad76b976f01882cf2a81129815ff9
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2024-08-14 (Wed, 14 Aug 2024)

  Changed paths:
    A LayoutTests/accessibility/animated-dropdown-expected.txt
    A LayoutTests/accessibility/animated-dropdown.html
    A LayoutTests/accessibility/mac/child-update-during-ax-request-expected.txt
    A LayoutTests/accessibility/mac/child-update-during-ax-request.html
    A LayoutTests/accessibility/resources/jquery-3.6.1.js
    M LayoutTests/platform/glib/TestExpectations
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

  Log Message:
  -----------
  REGRESSION(281440 at main): Web content can become inaccessible after dynamic webpage changes
rdar://133477773
https://bugs.webkit.org/show_bug.cgi?id=277817

Reviewed by Andres Gonzalez and Chris Fleizach.

In http://commits.webkit.org/281440@main, we fixed a bug where `AXIsolatedTree::m_protectedFromDeletionIDs` was
cleared too early, which in turn caused objects to be incorrectly removed from `AXIsolatedTree::m_nodeMap`, eventually
breaking the accessibility tree. We fixed this by moving:

`m_pendingProtectedFromDeletionIDs.formUnion(std::exchange(m_protectedFromDeletionIDs, { }))`

from `queueRemovalsLocked` to `queueAppendsAndRemovals` (which should happen once per tree-update cycle, making it the
right time to clear this list).

This caused a new bug, however, in this sequence:

  1. The main-thread starts a tree update via `AXIsolatedTree::updateChildren`.

  2. In doing so, we call `queueRemovalsLocked`, i.e. because an object lost a child (but that child isn't deleted, just
     has a different parent, so we mark it as protected on the main-thread in `m_protectedFromDeletionIDs`).

  3. We call `queueRemovalsLocked` for that child and any others no longer children of the original object. After
     281440 at main, we no longer sync protected IDs in this function.

  4. Before the full main-thread tree-update (`AXIsolatedTree::updateChildren`) is finished, resulting in a call to
     `queueAppendsAndRemovals`, the secondary thread processes a request from an AT, causing `AXIsolatedTree::applyPendingChanges`
     to run on the secondary thread (as expected to ensure we service the request with the most up-to-date information).

  5. We delete the re-parented child, and any of its descendants, from the accessibility thread data structures because
     we failed to protect it.

  6. `queueAppendsAndRemovals` eventually finishes on the main-thread, and we sync the protected objects to the secondary
     thread, but at that point it's too late.

  7. The accessibility tree is now broken, with random objects missing entirely.

This patch fixes this by changing `queueRemovalsLocked` (called by `queueAppendsAndRemovals`) to sync protected object
IDs if necessary, and changes `queueAppendsAndRemovals` to clear the list of protected objects, fixing both the bug
addressed by http://commits.webkit.org/281440@main, and the bug it caused.

Because 281440 at main caused a bug, it was reverted in https://github.com/WebKit/WebKit/pull/31917.
This PR brings back the test 281440 at main added (animated-dropdown.html), and adds a new test (child-update-during-ax-request.html)
that would've caught the bug 281440 at main introduced.

* LayoutTests/accessibility/animated-dropdown-expected.txt: Added.
* LayoutTests/accessibility/animated-dropdown.html: Added.
* LayoutTests/accessibility/mac/child-update-during-ax-request-expected.txt: Added.
* LayoutTests/accessibility/mac/child-update-during-ax-request.html: Added.
* LayoutTests/accessibility/resources/jquery-3.6.1.js: Added.
* LayoutTests/platform/glib/TestExpectations: Skip accessibility/animated-dropdown.html.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::queueRemovalsLocked):
(WebCore::AXIsolatedTree::queueAppendsAndRemovals):
(WebCore::AXIsolatedTree::collectNodeChangesForSubtree):
(WebCore::AXIsolatedTree::updateChildren):
(WebCore::AXIsolatedTree::protectFromDeletion):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

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



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