Reland: Fix issue with accessibility events on hidden nodes

Original CL: http://crrev.com/c/2134848
Revert: http://crrev.com/c/2138415

Landed at the same time as another change that affected test
expectations, just needed to be rebaselined and landed again.

This is an issue with AXObjects that are ignored and not
included in the accessibility tree. When we fire an event
on such a node, the serializer owned by
RenderAccessibilityImpl first walks up to find a common
ancestor that is in the tree, and then serializes that
node, even if it was already serialized. It breaks the
mechanism that avoids re-serializing nodes that were
already just serialized.

In extreme circumstances, this led to a bug where the
size of the AXTreeUpdate for a large table grew to 200MB
for one particular website, even though the resulting
AXTree had only around 2MB worth of data.

The fix is simple: when a node changes, only fire a
notification on the nearest ancestor that's in the tree.
That way the serializer can avoid redundantly processing
the same node more than once.

Covered by a new test and a DCHECK that asserts that we
should never get an AXTreeUpdate with more nodes than the
size of the resulting tree. Without the new fix, the test
fails with an AXTreeUpdate containing 1000 nodes with a
tree of size 10.

Bug: 1066880
Tbr: Adam Ettenberger <Adam.Ettenberger@microsoft.com>
Tbr: Aaron Leventhal <aleventhal@chromium.org>
Relnotes: N/A
Change-Id: I485e8d98d8fe9638706e26ceff642664a3fa6e3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140033Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Adam Ettenberger <Adam.Ettenberger@microsoft.com>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757221}

content/browser/accessibility/browser_accessibility_manager.cc

@@ -405,6 +405,13 @@ bool BrowserAccessibilityManager::OnAccessibilityEvents(
       }
       return false;
     }
+
+    // It's a bug if we got an update containing more nodes than
+    // the size of the resulting tree. If Unserialize succeeded that
+    // means a node just got repeated or something harmless like that,
+    // but it should still be investigated and could be the sign of a
+    // performance issue.
+    DCHECK_LE(int{tree_update.nodes.size()}, ax_tree()->size());
   }
 
   // If this page is hidden by an interstitial, suppress all events.

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -2382,6 +2382,10 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, DISABLED_XmlInIframeCrash) {
   RunRegressionTest(FILE_PATH_LITERAL("xml-in-iframe-crash.html"));
 }
 
+IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, HiddenTable) {
+  RunRegressionTest(FILE_PATH_LITERAL("hidden-table.html"));
+}
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
                        LanguageDetectionLangAttribute) {
   RunLanguageDetectionTest(FILE_PATH_LITERAL("lang-attribute.html"));

content/renderer/accessibility/render_accessibility_impl.cc

@@ -802,6 +802,14 @@ void RenderAccessibilityImpl::SendPendingAccessibilityEvents() {
     // parts of the code as well, so we need to ensure the object still exists.
     if (!obj.UpdateLayoutAndCheckValidity())
       continue;
+
+    // If the object in question is not included in the tree, get the
+    // nearest ancestor that is (ParentObject() will do this for us).
+    // Otherwise this can lead to the serializer doing extra work because
+    // the object won't be in |already_serialized_ids|.
+    if (!obj.AccessibilityIsIncludedInTree())
+      obj = obj.ParentObject();
+
     if (already_serialized_ids.find(obj.AxID()) != already_serialized_ids.end())
       continue;
 

content/test/data/accessibility/regression/hidden-table-expected-blink.txt

+rootWebArea
+++genericContainer ignored
+++++genericContainer ignored
+++++++table
+++++++++row
+++++++++++columnHeader name='Header 1'
+++++++++++++staticText name='Header 1'
+++++++++++++++inlineTextBox name='Header 1'
+++++++++++columnHeader name='Header 2'
+++++++++++++staticText name='Header 2'
+++++++++++++++inlineTextBox name='Header 2'
+++++++genericContainer
+++++++++staticText name='Done'
+++++++++++inlineTextBox name='Done'

content/test/data/accessibility/regression/hidden-table.html

+<!doctype html>
+<!--
+@WAIT-FOR:Done
+-->
+<body>
+  <table>
+    <thead>
+      <tr>
+        <th>Header 1</th>
+        <th>Header 2</th>
+      </tr>
+    </thead>
+    <tbody hidden>
+    </tbody>
+    <tfoot>
+    </tfoot>
+  </table>
+
+  <script>
+    function addRow(container) {
+        let row = document.createElement('tr');
+        container.appendChild(row);
+        for (let j = 0; j < 100; j++) {
+            let cell = document.createElement('td');
+            cell.innerHTML = Math.random();
+            row.appendChild(cell);
+            cell.tabIndex = -1;
+        }
+    }
+
+    setTimeout(() => {
+        // Now add a bunch of rows to the tbody, which is hidden.
+        // Every one of those added cells changes its tabIndex, which
+        // triggers a call to MarkAXObjectDirty on a node that's not
+        // included in the accessibility tree. That caused a bug where
+        // all of the real nodes of the table keep getting re-serialized.
+        // This is now caught by a DCHECK in BrowserAccessibilityManager
+        // that an AXTreeUpdate shouldn't be larger than the resulting tree.
+        for (let i = 0; i < 10; i++) {
+            addRow(document.querySelector('tbody'));
+        }
+
+        // Finally, clear out all of those extra rows and finish
+        // the test.
+        setTimeout(() => {
+            document.querySelector('tbody').innerHTML = '';
+            let done = document.createElement('div');
+            done.innerHTML = 'Done';
+            document.body.appendChild(done);
+        }, 10);
+    }, 10);
+  </script>
+</body>