ClearChildren should only detach children who point to that parent.

We want to to allow relations such as labelled-by and described-by
to reference elements that are inert.

There seems to be precedent for a node having a child with a parent
other than itself (see ax_node_object.cc:2447). With this in mind,
when a node node is being cleared, it should only clear its child's
parent pointer if it points to itself(the parent currently
clearing its children).

Example:
node a: children = { b, c }, parent = null
node b: children = { c }, parent = a
node c: children = { g, h, i }, parent = a

In this case, node c is both a child of the a (the root) and b.
If we clear node b, we should not disconnect node c, from a.

The tree would look like this:
a
|____
|   |
b   c
    |________
    |   |   |
    g   h   i

Where node b would have indirect_children = {c}

Patchset 1 reverts the mitigation that was checked in here:
commit 5ef90f37
https://chromium-review.googlesource.com/c/chromium/src/+/1814460

Bug: 996992
Change-Id: I9b9cbc10aea473270178f15f283a1bacfa71b971
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838473
Commit-Queue: Jacques Newman <janewman@microsoft.com>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706627}

content/browser/accessibility/dump_accessibility_browsertest_base.cc

@@ -145,6 +145,7 @@ DumpAccessibilityTestBase::DumpUnfilteredAccessibilityTreeAsString() {
 void DumpAccessibilityTestBase::ParseHtmlForExtraDirectives(
     const std::string& test_html,
     std::vector<std::string>* wait_for,
+    std::vector<std::string>* execute,
     std::vector<std::string>* run_until,
     std::vector<std::string>* default_action_on) {
   for (const std::string& line : base::SplitString(
@@ -154,6 +155,7 @@ void DumpAccessibilityTestBase::ParseHtmlForExtraDirectives(
     const std::string& deny_str = formatter_->GetDenyString();
     const std::string& deny_node_str = formatter_->GetDenyNodeString();
     const std::string& wait_str = "@WAIT-FOR:";
+    const std::string& execute_str = "@EXECUTE-AND-WAIT-FOR:";
     const std::string& until_str = "@RUN-UNTIL-EVENT:";
     const std::string& default_action_on_str = "@DEFAULT-ACTION-ON:";
     if (base::StartsWith(line, allow_empty_str, base::CompareCase::SENSITIVE)) {
@@ -181,6 +183,9 @@ void DumpAccessibilityTestBase::ParseHtmlForExtraDirectives(
       }
     } else if (base::StartsWith(line, wait_str, base::CompareCase::SENSITIVE)) {
       wait_for->push_back(line.substr(wait_str.size()));
+    } else if (base::StartsWith(line, execute_str,
+                                base::CompareCase::SENSITIVE)) {
+      execute->push_back(line.substr(execute_str.size()));
     } else if (base::StartsWith(line, until_str,
                                 base::CompareCase::SENSITIVE)) {
       run_until->push_back(line.substr(until_str.size()));
@@ -267,13 +272,14 @@ void DumpAccessibilityTestBase::RunTestForPlatform(
 
   // Parse filters and other directives in the test file.
   std::vector<std::string> wait_for;
+  std::vector<std::string> execute;
   std::vector<std::string> run_until;
   std::vector<std::string> default_action_on;
   property_filters_.clear();
   node_filters_.clear();
   formatter_->AddDefaultFilters(&property_filters_);
   AddDefaultFilters(&property_filters_);
-  ParseHtmlForExtraDirectives(html_contents, &wait_for, &run_until,
+  ParseHtmlForExtraDirectives(html_contents, &wait_for, &execute, &run_until,
                               &default_action_on);
 
   // Get the test URL.
@@ -386,6 +392,35 @@ void DumpAccessibilityTestBase::RunTestForPlatform(
   // Call the subclass to dump the output.
   std::vector<std::string> actual_lines = Dump(run_until);
 
+  // Execute and wait for specified string
+  for (const auto& function_name : execute) {
+    DLOG(INFO) << "executing: " << function_name;
+    base::Value result =
+        ExecuteScriptAndGetValue(web_contents->GetMainFrame(), function_name);
+    const std::string& str = result.is_string() ? result.GetString() : "";
+    // If no string is specified, do not wait.
+    bool wait_for_string = str != "";
+    while (wait_for_string) {
+      // Loop until specified string is found.
+      base::string16 tree_dump = DumpUnfilteredAccessibilityTreeAsString();
+      if (base::UTF16ToUTF8(tree_dump).find(str) != std::string::npos) {
+        wait_for_string = false;
+        // Append an additional dump if the specified string was found.
+        std::vector<std::string> additional_dump = Dump(run_until);
+        actual_lines.emplace_back("=== Start Continuation ===");
+        actual_lines.insert(actual_lines.end(), additional_dump.begin(),
+                            additional_dump.end());
+        break;
+      }
+      // Block until the next accessibility notification in any frame.
+      VLOG(1) << "Still waiting on this text to be found: " << str;
+      VLOG(1) << "Waiting until the next accessibility event";
+      AccessibilityNotificationWaiter accessibility_waiter(
+          web_contents, ui::AXMode(), ax::mojom::Event::kNone);
+      accessibility_waiter.WaitForNotification();
+    }
+  }
+
   // Validate against the expectation file.
   bool matches_expectation = test_helper.ValidateAgainstExpectation(
       file_path, expected_file, actual_lines, *expected_lines);

content/browser/accessibility/dump_accessibility_browsertest_base.h

@@ -90,6 +90,7 @@ class DumpAccessibilityTestBase : public ContentBrowserTest,
   // @WAIT-FOR: directives.
   void ParseHtmlForExtraDirectives(const std::string& test_html,
                                    std::vector<std::string>* wait_for,
+                                   std::vector<std::string>* execute,
                                    std::vector<std::string>* run_until,
                                    std::vector<std::string>* default_action_on);
 

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -1836,6 +1836,10 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityOptgroup) {
   RunHtmlTest(FILE_PATH_LITERAL("optgroup.html"));
 }
 
+IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityOpenModal) {
+  RunHtmlTest(FILE_PATH_LITERAL("open-modal.html"));
+}
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
                        AccessibilityOptionindatalist) {
   RunHtmlTest(FILE_PATH_LITERAL("option-in-datalist.html"));

content/test/data/accessibility/html/modal-dialog-opened-expected-android.txt

 android.webkit.WebView focusable scrollable
+++android.view.View
 ++android.app.Dialog role_description='dialog'
 ++++android.view.View name='The dialog subtree should be the only text content in the accessibility tree. '
 ++++android.view.View role_description='link' clickable focusable focused link name='Link inside the dialog.'
-++android.view.View

content/test/data/accessibility/html/modal-dialog-opened-expected-auralinux.txt

 [document web]
+++[section]
 ++[dialog] modal
 ++++[text] name='The dialog subtree should be the only text content in the accessibility tree. '
 ++++[link] name='Link inside the dialog.'
 ++++++[text] name='Link inside the dialog.'
-++[section]

content/test/data/accessibility/html/modal-dialog-opened-expected-blink.txt

 rootWebArea
+++genericContainer ignored invisible
+++++section ignored invisible
+++++++popUpButton collapsed ignored invisible value='This should be pruned out of the tree.'
+++++button ignored invisible name='Choose File' value='No file chosen'
+++genericContainer ignored invisible
+++dialog ignored invisible
+++genericContainer
 ++dialog
 ++++staticText name='The dialog subtree should be the only text content in the accessibility tree. '
 ++++++inlineTextBox name='The dialog subtree should be the only text content in the accessibility tree. '
 ++++link name='Link inside the dialog.'
 ++++++staticText name='Link inside the dialog.'
 ++++++++inlineTextBox name='Link inside the dialog.'
-++genericContainer

content/test/data/accessibility/html/modal-dialog-opened-expected-mac.txt

 AXWebArea
+++AXGroup
 ++AXGroup AXSubrole=AXApplicationDialog
 ++++AXStaticText AXValue='The dialog subtree should be the only text content in the accessibility tree. '
 ++++AXLink AXTitle='Link inside the dialog.'
 ++++++AXStaticText AXValue='Link inside the dialog.'
-++AXGroup

content/test/data/accessibility/html/modal-dialog-opened-expected-uia-win.txt

 document
+++group
 ++dialog Window.IsModal=true
 ++++description Name='The dialog subtree should be the only text content in the accessibility tree. '
 ++++link Name='Link inside the dialog.'
-++group

content/test/data/accessibility/html/modal-dialog-opened-expected-win.txt

 ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE
+++IA2_ROLE_SECTION
 ++ROLE_SYSTEM_DIALOG IA2_STATE_MODAL
 ++++ROLE_SYSTEM_STATICTEXT name='The dialog subtree should be the only text content in the accessibility tree. '
 ++++ROLE_SYSTEM_LINK name='Link inside the dialog.' FOCUSABLE
 ++++++ROLE_SYSTEM_STATICTEXT name='Link inside the dialog.'
-++IA2_ROLE_SECTION

content/test/data/accessibility/html/modal-dialog-stack-expected-android.txt

 android.webkit.WebView focusable focused scrollable
+++android.view.View
 ++android.app.Dialog role_description='dialog'
 ++++android.view.View name='This is the now active dialog. Of course it should be in the tree. '
 ++++android.widget.Button role_description='button' clickable focusable name='This is in the active dialog and should be in the tree.'
-++android.view.View

content/test/data/accessibility/html/modal-dialog-stack-expected-auralinux.txt

 [document web]
+++[section]
 ++[dialog] modal
 ++++[text] name='This is the now active dialog. Of course it should be in the tree. '
 ++++[push button] name='This is in the active dialog and should be in the tree.'
-++[section]

content/test/data/accessibility/html/modal-dialog-stack-expected-blink.txt

 rootWebArea
+++genericContainer ignored invisible
+++++section ignored invisible
+++++++dialog ignored invisible name=' This was the top dialog and should not be in the tree. '
+++++++popUpButton collapsed ignored invisible value='This should be pruned out of the tree.'
+++++button ignored invisible name='This button should not be in the tree.'
+++++dialog ignored invisible name=' This was the middle dialog and should not be in the tree. '
+++genericContainer ignored invisible
+++dialog ignored invisible
+++genericContainer
 ++dialog
 ++++staticText name='This is the now active dialog. Of course it should be in the tree. '
 ++++++inlineTextBox name='This is the now active dialog. Of course it should be in the tree. '
 ++++button name='This is in the active dialog and should be in the tree.'
-++genericContainer

content/test/data/accessibility/html/modal-dialog-stack-expected-mac.txt

 AXWebArea
+++AXGroup
 ++AXGroup AXSubrole=AXApplicationDialog
 ++++AXStaticText AXValue='This is the now active dialog. Of course it should be in the tree. '
 ++++AXButton AXTitle='This is in the active dialog and should be in the tree.'
-++AXGroup

content/test/data/accessibility/html/modal-dialog-stack-expected-uia-win.txt

 document
+++group
 ++dialog Window.IsModal=true
 ++++description Name='This is the now active dialog. Of course it should be in the tree. '
 ++++button Name='This is in the active dialog and should be in the tree.'
-++group

content/test/data/accessibility/html/modal-dialog-stack-expected-win.txt

 ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE
+++IA2_ROLE_SECTION
 ++ROLE_SYSTEM_DIALOG IA2_STATE_MODAL
 ++++ROLE_SYSTEM_STATICTEXT name='This is the now active dialog. Of course it should be in the tree. '
 ++++ROLE_SYSTEM_PUSHBUTTON name='This is in the active dialog and should be in the tree.' FOCUSABLE
-++IA2_ROLE_SECTION

content/test/data/accessibility/html/open-modal-expected-blink.txt

+rootWebArea
+++genericContainer ignored
+++++staticText name='some text'
+++++++inlineTextBox name='some text'
+++++genericContainer ignored
+=== Start Continuation ===
+rootWebArea
+++genericContainer ignored invisible
+++++genericContainer ignored invisible
+++genericContainer
+++dialog
+++++genericContainer name='Dialog Text'
+++++++staticText name='Dialog Text'
+++++++++inlineTextBox name='Dialog Text'
+=== Start Continuation ===
+rootWebArea
+++genericContainer ignored invisible
+++++genericContainer ignored invisible
+++genericContainer
+++dialog
+++++genericContainer name='Done'
+++++++staticText name='Done'
+++++++++inlineTextBox name='Done'

content/test/data/accessibility/html/open-modal.html

+<!--
+@EXECUTE-AND-WAIT-FOR:open_modal()
+@EXECUTE-AND-WAIT-FOR:remove_span()
+This regression test ensures that the dialog is included in the tree.
+There was a bug where ClearChildren would disconnect indirect children from
+their parent. crbug: 996992
+-->
+<!DOCTYPE html>
+<html>
+
+<span>some text</span>
+<div>
+  <dialog>
+    <div id="dialog-text" tabindex="0">Dialog Text</div>
+  </dialog>
+</div>
+
+
+<script>
+  function open_modal() {
+    document.querySelector("dialog").showModal();
+    return "Dialog Text";
+  }
+  function remove_span() {
+    // Cause the root to be updated.
+    let span = document.querySelector("span");
+    span.parentElement.removeChild(span);
+
+    document.getElementById("dialog-text").innerText = "Done";
+    return "Done";
+  }
+</script>
+
+</html>
\ No newline at end of file

content/test/data/accessibility/readme.md

@@ -118,6 +118,14 @@ system will keep blocking until that text appears.
 You can add as many `@WAIT-FOR:` directives as you want, the test won't finish
 until all strings appear.
 
+You may also want to execute script and then capture a dump. Rather than use
+`setTimeout` and `@WAIT-FOR:`, consider using the `@EXECUTE-AND-WAIT-FOR:`
+directive. This directive expects a javascript function that returns a string to
+wait for. If a string is not returned, the tree dumper will not wait.
+`@EXECUTE-AND-WAIT-FOR:` directives are executed in order, after the document is
+ready and all `@WAIT-FOR:` strings have been found.
+Example: `@EXECUTE-AND-WAIT-FOR: foo()`
+
 Or, you may need to write an event test that keeps dumping events until a
 specific event line. In this case, use `@RUN-UNTIL-EVENT` with a substring that
 should occur in the event log, e.g.,

third_party/blink/renderer/modules/accessibility/ax_object.cc

@@ -1212,11 +1212,6 @@ bool AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree() const {
   if (!GetNode())
     return false;
 
-  // Disallow inert nodes from the tree to ensure the dialog is always the
-  // first child of the root.
-  if (GetNode()->IsInert())
-    return false;
-
   // If the node is part of the user agent shadow dom, or has the explicit
   // internal Role::kIgnored, they aren't interesting for paragraph navigation
   // or LabelledBy/DescribedBy relationships.
@@ -2562,8 +2557,10 @@ void AXObject::UpdateChildrenIfNecessary() {
 
 void AXObject::ClearChildren() {
   // Detach all weak pointers from objects to their parents.
-  for (const auto& child : children_)
-    child->DetachFromParent();
+  for (const auto& child : children_) {
+    if (child->parent_ == this)
+      child->DetachFromParent();
+  }
 
   children_.clear();
   have_children_ = false;