From 122854807a20e65780bf30a0fd52d0da03c4ba76 Mon Sep 17 00:00:00 2001
From: Aaron Leventhal <aleventhal@chromium.org>
Date: Tue, 30 May 2023 21:29:01 +0000
Subject: [PATCH] Merge M115: Revert "Reland "Improve slot a11y
 invalidation/cleanup""

This reverts commit f7485917cc6af96436ee001bcb26fb304f1bf1e7.

Reason for revert: performance regression https://crbug.com/1447357

Original change's description:
> Reland "Improve slot a11y invalidation/cleanup"
>
> This is a reland of commit dd9058a22667078a2378ea9b4d0c239dded5387a
>
> Reason for revert: broke Fuchsia test expectations.
> https://ci.chromium.org/p/chromium/builders/ci/fuchsia-arm64-cast-receiver-rel/3859. Fuchsia should not actually be picking up ignored nodes at all, and therefore it should not have regressed. I've filed a bug on Fuchsia to change that behavior: crbug.com/1420272
>
> Patchset 1 contains the originally-landed code.
>
> Original change's description:
> > Improve slot a11y invalidation/cleanup
> >
> > Remove obscure logic related to determining if a slot is relevant, and just treat them all as relevant. I believe this was required to pass some PDF accessibility tests in the pass, but no longer seems to be. This causes a couple of changes to Blink expectations, but because the irrelevant slots are still hidden/ignored, the platform expectations are not changing, which is what's really important.
> >
> > As a result, we no longer leave extra objects in AXObjectCache or serializer when an there are changes related to slots. This is verified by a follow-up, CL:4185240, and this CL is necessary to pass the checks added there.
> >
> > Bug: None
> >
> > Change-Id: If787fcfc79a3568ffacc262d29f7cc4a5d89dbba
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4295193
> > Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
> > Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1110744}
>
> Bug: None
> Change-Id: Ic323ad86a57c6254659660979311636ff3bd213f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4294848
> Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
> Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1111125}

(cherry picked from commit 71b3160b0f8bb4e6cc21b2d4614126982a5a4625)

Fixed: 1447357
Change-Id: Ia8d5463baa90f831f30d41e4bfcf8d314ec2953b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4566753
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1149384}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4573789
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#147}
Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114}
---
 .../custom-element-hidden-expected-blink.txt  |  5 +-
 .../html/meter-expected-blink.txt             |  2 -
 .../html/meter-expected-fuchsia.txt           |  4 +-
 .../html/popover-api-expected-blink.txt       |  3 -
 .../html/selectmenu-expected-blink.txt        |  6 +-
 .../html/selectmenu-expected-fuchsia.txt      |  4 --
 .../title-in-shadow-expected-blink.txt        |  1 -
 .../modules/accessibility/ax_node_object.cc   | 15 +++-
 .../accessibility/ax_object_cache_impl.cc     | 69 +++++++++++++++++++
 .../accessibility/ax_object_cache_impl.h      |  1 +
 10 files changed, 85 insertions(+), 25 deletions(-)

diff --git a/content/test/data/accessibility/html/custom-element-hidden-expected-blink.txt b/content/test/data/accessibility/html/custom-element-hidden-expected-blink.txt
index f2f68f3dc68e2a..9db06e2f2a933c 100644
--- a/content/test/data/accessibility/html/custom-element-hidden-expected-blink.txt
+++ b/content/test/data/accessibility/html/custom-element-hidden-expected-blink.txt
@@ -5,7 +5,4 @@ rootWebArea htmlTag='#document'
 ++++++genericContainer ignored invisible htmlTag='div'
 ++++++++genericContainer ignored invisible htmlTag='my-element'
 ++++++++++genericContainer ignored invisible htmlTag='div'
-++++++++++++genericContainer ignored invisible htmlTag='slot'
-++++++++++++++genericContainer ignored invisible htmlTag='span'
-++++++++++++++++staticText ignored invisible name='Slot contents'
-++++++button htmlTag='button' name='Done'
\ No newline at end of file
+++++++button htmlTag='button' name='Done'
diff --git a/content/test/data/accessibility/html/meter-expected-blink.txt b/content/test/data/accessibility/html/meter-expected-blink.txt
index 6c2d34a473e009..3396f991a8bbe0 100644
--- a/content/test/data/accessibility/html/meter-expected-blink.txt
+++ b/content/test/data/accessibility/html/meter-expected-blink.txt
@@ -6,5 +6,3 @@ rootWebArea
 ++++++++++genericContainer
 ++++++++++++genericContainer ignored
 ++++++++genericContainer ignored invisible
-++++++++++genericContainer ignored invisible
-++++++++++++staticText ignored invisible name='2 out of 10'
\ No newline at end of file
diff --git a/content/test/data/accessibility/html/meter-expected-fuchsia.txt b/content/test/data/accessibility/html/meter-expected-fuchsia.txt
index 8a1d030cd6a02f..ae7808c7fec696 100644
--- a/content/test/data/accessibility/html/meter-expected-fuchsia.txt
+++ b/content/test/data/accessibility/html/meter-expected-fuchsia.txt
@@ -5,6 +5,4 @@ UNKNOWN focusable has_input_focus
 ++++++++UNKNOWN
 ++++++++++UNKNOWN
 ++++++++++++UNKNOWN hidden
-++++++++UNKNOWN hidden
-++++++++++UNKNOWN hidden
-++++++++++++STATIC_TEXT hidden label='2 out of 10'
+++++++++UNKNOWN hidden
\ No newline at end of file
diff --git a/content/test/data/accessibility/html/popover-api-expected-blink.txt b/content/test/data/accessibility/html/popover-api-expected-blink.txt
index 5b1d263225402b..f609289499b1f9 100644
--- a/content/test/data/accessibility/html/popover-api-expected-blink.txt
+++ b/content/test/data/accessibility/html/popover-api-expected-blink.txt
@@ -10,9 +10,6 @@ rootWebArea
 ++++++++genericContainer ignored invisible
 ++++++++++staticText ignored invisible name='Listbox role on ul'
 ++++++details ignored invisible
-++++++++genericContainer ignored invisible
-++++++++++disclosureTriangle ignored invisible
-++++++++++++staticText ignored invisible name='summary/details popover'
 ++++++button collapsed name='Button pointing to hidden popover'
 ++++++++staticText name='Button pointing to hidden popover'
 ++++++++++inlineTextBox name='Button pointing to hidden popover'
diff --git a/content/test/data/accessibility/html/selectmenu-expected-blink.txt b/content/test/data/accessibility/html/selectmenu-expected-blink.txt
index e542bb45417b33..065b897e003f7f 100644
--- a/content/test/data/accessibility/html/selectmenu-expected-blink.txt
+++ b/content/test/data/accessibility/html/selectmenu-expected-blink.txt
@@ -12,14 +12,10 @@ rootWebArea focusable
 ++++++++++++++genericContainer ignored
 ++++++++genericContainer ignored
 ++++++++++listBox ignored invisible
-++++++++++++genericContainer ignored invisible
-++++++++++++++listBoxOption focusable ignored invisible
-++++++++++++++listBoxOption focusable ignored invisible
-++++++++++++++listBoxOption focusable ignored invisible
 ++++++genericContainer
 ++++++++genericContainer ignored
 ++++++++++comboBoxMenuButton collapsed value='Option 1' haspopup=listbox
 ++++++++++++staticText name='Custom selectmenu button'
 ++++++++++++++inlineTextBox name='Custom selectmenu button'
 ++++++++genericContainer ignored
-++++++++++listBox ignored invisible
\ No newline at end of file
+++++++++++listBox ignored invisible
diff --git a/content/test/data/accessibility/html/selectmenu-expected-fuchsia.txt b/content/test/data/accessibility/html/selectmenu-expected-fuchsia.txt
index 64300cf2491c38..802f8fa70fda9f 100644
--- a/content/test/data/accessibility/html/selectmenu-expected-fuchsia.txt
+++ b/content/test/data/accessibility/html/selectmenu-expected-fuchsia.txt
@@ -12,10 +12,6 @@ UNKNOWN focusable has_input_focus
 ++++++++++++++UNKNOWN hidden
 ++++++++UNKNOWN hidden
 ++++++++++UNKNOWN hidden
-++++++++++++UNKNOWN hidden
-++++++++++++++UNKNOWN hidden focusable actions='{DEFAULT}'
-++++++++++++++UNKNOWN hidden focusable actions='{DEFAULT}'
-++++++++++++++UNKNOWN hidden focusable actions='{DEFAULT}'
 ++++++UNKNOWN
 ++++++++UNKNOWN hidden
 ++++++++++UNKNOWN actions='{DEFAULT}' value='Option 1'
diff --git a/content/test/data/accessibility/regression/title-in-shadow-expected-blink.txt b/content/test/data/accessibility/regression/title-in-shadow-expected-blink.txt
index fd7f44e4232668..099b17beb2e753 100644
--- a/content/test/data/accessibility/regression/title-in-shadow-expected-blink.txt
+++ b/content/test/data/accessibility/regression/title-in-shadow-expected-blink.txt
@@ -6,4 +6,3 @@ rootWebArea name=''
 ++++++++++genericContainer
 ++++++++++++genericContainer ignored
 ++++++++genericContainer ignored invisible
-++++++++++genericContainer ignored invisible
\ No newline at end of file
diff --git a/third_party/blink/renderer/modules/accessibility/ax_node_object.cc b/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
index 99c6b2e2373e4d..67b1c83b6c61b7 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
@@ -3212,9 +3212,18 @@ String AXNodeObject::GetValueForControl() const {
     if (auto* select_menu = HTMLSelectMenuElement::OwnerSelectMenu(node)) {
       DCHECK(RuntimeEnabledFeatures::HTMLSelectMenuElementEnabled());
       if (HTMLOptionElement* selected = select_menu->selectedOption()) {
-        if (selected->firstChild()) {
-          return selected->textContent();
-        }
+        // TODO(accessibility) Because these <option> elements can contain
+        // anything, we need to create an AXObject for the selected option, and
+        // use ax_selected_option->ComputedName(). However, for now, the
+        // AXObject is not created because AXObject::IsRelevantSlotElement()
+        // returns false for the invisible slot parent. Also, strangely,
+        // selected->innerText()/GetInnerTextWithoutUpdate() are returning "".
+        // See the following content_browsertest:
+        // All/DumpAccessibilityTreeTest.AccessibilitySelectMenu/blink.
+        // TODO(crbug.com/1401767): DCHECK fails with synchronous serialization.
+        DCHECK(selected->firstChild())
+            << "There is a selected option but it has no DOM children.";
+        return selected->textContent();
       }
       return String();
     }
diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
index e89ff895457578..8dab825d576d12 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
@@ -515,6 +515,12 @@ bool IsSubtreePrunedForAccessibility(const Element* node) {
       return true;
   }
 
+  if (const HTMLSlotElement* slot =
+          ToHTMLSlotElementIfSupportsAssignmentOrNull(node)) {
+    if (!AXObjectCacheImpl::IsRelevantSlotElement(*slot))
+      return true;
+  }
+
   // <optgroup> is irrelevant inside of a <select> menulist.
   if (auto* opt_group = DynamicTo<HTMLOptGroupElement>(node)) {
     if (auto* select = opt_group->OwnerSelectElement()) {
@@ -1130,6 +1136,57 @@ bool AXObjectCacheImpl::ShouldCreateAXMenuListFor(LayoutObject* layout_object) {
   return false;
 }
 
+// static
+bool AXObjectCacheImpl::IsRelevantSlotElement(const HTMLSlotElement& slot) {
+  // A <slot> descendant of a node that is still in the DOM but no longer
+  // rendered will return true for Node::isConnected() and false for
+  // AXObject::IsDetached(). But from the perspective of platform ATs, this
+  // subtree is not connected and is detached unless it is canvas fallback
+  // content. In order to detect this condition, we look to the first non-slot
+  // parent. If it has a layout object, the <slot>'s contents are rendered.
+  // If it doesn't, but it's in the canvas subtree, those contents should be
+  // treated as canvas fallback content.
+  //
+  // The alternative way to determine whether the <slot> is still relevant for
+  // rendering is to iterate FlatTreeTraversal::Parent until you get to the last
+  // parent, and see if it's a document. If it is not a document, then it is not
+  // relevant. This seems much slower than just checking GetLayoutObject() as it
+  // needs to iterate the parent chain. However, checking GetLayoutObject()
+  // could produce null in the case of something that is
+  // content-visibility:auto. This means that any slotted content inside
+  // content-visibility:auto may be removed from the AX tree depending on
+  // whether it was recently rendered.
+  //
+  // TODO(accessibility) This fails for the web test
+  // detach-locked-slot-children-crash.html with --force-renderer-accessibility.
+  // See web_tests/FlagExpectations/force-renderer-accessibility.
+  // There should be a better way to accomplish this.
+  // Could a new function be added to the slot element?
+  const Node* parent = LayoutTreeBuilderTraversal::Parent(slot);
+  if (const HTMLSlotElement* parent_slot =
+          ToHTMLSlotElementIfSupportsAssignmentOrNull(parent)) {
+    return AXObjectCacheImpl::IsRelevantSlotElement(*parent_slot);
+  }
+
+  if (parent && parent->GetLayoutObject())
+    return true;
+
+  const Element* parent_element = DynamicTo<Element>(parent);
+  if (!parent_element)
+    return false;
+
+  // Authors can include elements as "Fallback content" inside a <canvas> in
+  // order to provide an alternative means to interact with the canvas using
+  // a screen reader. Those should always be included.
+  if (parent_element->IsInCanvasSubtree())
+    return true;
+
+  // LayoutObject::CreateObject() will not create an object for elements
+  // with display:contents. If we do not include a <slot> for that reason,
+  // any descendants will be not be included in the accessibility tree.
+  return parent_element->HasDisplayContentsStyle();
+}
+
 // static
 bool AXObjectCacheImpl::IsRelevantPseudoElement(const Node& node) {
   DCHECK(node.IsPseudoElement());
@@ -3826,6 +3883,18 @@ AXObject* AXObjectCacheImpl::GetSerializationTarget(AXObject* obj) {
     return nullptr;
   }
 
+  // A <slot> descendant of a node that is still in the DOM but no longer
+  // rendered will return true for Node::isConnected() and false for
+  // AXObject::IsDetached(). But from the perspective of platform ATs, this
+  // subtree is not connected and is detached.
+  // TODO(accessibility): The relevance check probably applies to all nodes
+  // not just slot elements.
+  if (const HTMLSlotElement* slot =
+          ToHTMLSlotElementIfSupportsAssignmentOrNull(obj->GetNode())) {
+    if (!AXObjectCacheImpl::IsRelevantSlotElement(*slot))
+      return nullptr;
+  }
+
   // Ensure still in tree.
   if (obj->IsMissingParent()) {
     // TODO(accessibility) Only needed because of <select> size changes.
diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
index 062198234e0f39..89529621d02b8a 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
+++ b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
@@ -440,6 +440,7 @@ class MODULES_EXPORT AXObjectCacheImpl
   static bool IsRelevantPseudoElement(const Node& node);
   static bool IsRelevantPseudoElementDescendant(
       const LayoutObject& layout_object);
+  static bool IsRelevantSlotElement(const HTMLSlotElement& slot);
 
   bool HasBeenDisposed() { return has_been_disposed_; }