Revert "Use PaintInvalidator to optimize AX bounding boxes"

This reverts commit fd1c45a7.

Reason for revert: Causing test AccessibilityActionBrowserTest.IncrementDecrementActions to fail. See for example https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/19799.

Original change's description:
> Use PaintInvalidator to optimize AX bounding boxes
> 
> Previously, any time there was a layout, the accessibility
> serialization code would scan every AXObject to see if any
> needed to update their bounding box.
> 
> Instead, replace this with a mechanism that uses
> PaintInvalidator to mark AXObjects that might have
> dirty bounding boxes.
> 
> Existing tests already provide some coverage; if you
> comment out the code in paint_invalidator.cc, a
> handful of browser tests fail.
> 
> To provide even more coverage, a debug-only check
> walks the entire tree and ensures that no nodes
> have incorrect bounding boxes. If you try commenting
> out the code in paint_invalidator.cc now, hundreds
> of browser tests fail. This provides some good
> confidence that it's working correctly.
> 
> Finally, add a new blink perf test demonstrating a
> simple scenario where this results in a dramatic
> speedup, just by focusing links in a document with a
> few thousand nodes.
> 
> Bug: 1109081
> AX-Relnotes: makes accessibility less sluggish on very large web pages
> 
> Change-Id: I51a89d0b37ff356c5443b324080acefe0e7f3fbf
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2319411
> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Nektarios Paisios <nektar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#792949}

TBR=dmazzoni@chromium.org,wangxianzhu@chromium.org,aboxhall@chromium.org,haraken@chromium.org,nektar@chromium.org

Change-Id: I3f44fc1c97dbe1690b881cb233b46f060f49ca72
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1109081
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2326560Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793039}

content/renderer/accessibility/render_accessibility_impl.cc

@@ -975,11 +975,15 @@ void RenderAccessibilityImpl::SendLocationChanges() {
   if (!root.UpdateLayoutAndCheckValidity())
     return;
 
-  blink::WebVector<WebAXObject> changed_bounds_objects;
-  root.GetAllObjectsWithChangedBounds(changed_bounds_objects);
-  for (const WebAXObject& obj : changed_bounds_objects) {
+  // Do a breadth-first explore of the whole blink AX tree.
+  base::queue<WebAXObject> objs_to_explore;
+  objs_to_explore.push(root);
+  while (objs_to_explore.size()) {
+    WebAXObject obj = objs_to_explore.front();
+    objs_to_explore.pop();
+
     // See if we had a previous location. If not, this whole subtree must
-    // be new, so no need to update.
+    // be new, so don't continue to explore this branch.
     int id = obj.AxID();
     if (!tree_source_->HasCachedBoundingBox(id))
       continue;
@@ -992,23 +996,6 @@ void RenderAccessibilityImpl::SendLocationChanges() {
 
     // Save the new location.
     tree_source_->SetCachedBoundingBox(id, new_location);
-  }
-
-#ifndef NDEBUG
-  // In the debug configuration, walk the entire tree and make sure no
-  // locations are incorrect.
-  base::queue<WebAXObject> objs_to_explore;
-  objs_to_explore.push(root);
-  while (objs_to_explore.size()) {
-    WebAXObject obj = objs_to_explore.front();
-    objs_to_explore.pop();
-
-    int id = obj.AxID();
-    if (tree_source_->HasCachedBoundingBox(id)) {
-      ui::AXRelativeBounds correct_location;
-      tree_source_->PopulateAXRelativeBounds(obj, &correct_location);
-      DCHECK_EQ(correct_location, tree_source_->GetCachedBoundingBox(id));
-    }
 
     // Explore children of this object.
     std::vector<WebAXObject> children;
@@ -1016,7 +1003,6 @@ void RenderAccessibilityImpl::SendLocationChanges() {
     for (WebAXObject& child : children)
       objs_to_explore.push(child);
   }
-#endif  // NDEBUG
 
   // Ensure that the number of cached bounding boxes doesn't exceed the
   // number of nodes in the tree, that would indicate the cache could

third_party/blink/perf_tests/accessibility/focus-links.html

-<!DOCTYPE html>
-<html>
-<body>
-<script src="../resources/runner.js"></script>
-
-<table id="testElement">
-  <tr>
-    <th>Sender</th>
-    <td>Message</td>
-  </tr>
-</table>
-
-<script>
-var isDone = false;
-var startTime;
-
-// Before the test starts, add 2000 rows to the table, something like a
-// message board with lots of replies on a long thread.
-
-let table = document.getElementById('testElement');
-for (let i = 0; i < 2000; i++) {
-    let tr = document.createElement('tr');
-    table.appendChild(tr);
-    let sender = document.createElement('td');
-    sender.innerHTML = 'user' + Math.floor(10000*Math.random());
-    tr.appendChild(sender);
-    let message = document.createElement('td');
-    message.innerHTML = '<div>Message content ' +
-        Math.floor(10000*Math.random()) + '</div>';
-    let link = document.createElement('a');
-    link.href = '#';
-    link.id = 'link' + i;
-    link.innerHTML = 'Reply';
-    message.appendChild(link);
-    tr.appendChild(message);
-}
-
-function runTest() {
-    if (startTime) {
-        PerfTestRunner.measureValueAsync(PerfTestRunner.now() - startTime);
-        PerfTestRunner.addRunTestEndMarker();
-    }
-    if (!isDone) {
-        PerfTestRunner.addRunTestStartMarker();
-        startTime = PerfTestRunner.now();
-
-        // Iterate over some of the links and focus each one with a different
-        // delay. Just focusing a link shouldn't incur a large cost,
-        // even if the page has a lot of elements.
-        for (let i = 0; i < 100; i++) {
-            window.setTimeout(() => {
-                document.getElementById('link' + i).focus();
-            }, 10 * i);
-        }
-
-        // Wait to allow the asynchronous accessibility code that's
-        // covered by traceEventsToMeasure to have a chance to run.
-        setTimeout(runTest, 1500);
-    }
-}
-
-PerfTestRunner.startMeasureValuesAsync({
-    description: 'Test accessibility performance when appending to a textarea.',
-    unit: 'ms',
-    done: function () {
-        isDone = true;
-    },
-    run: function() {
-        runTest();
-    },
-    iterationCount: 6,
-    tracingCategories: 'accessibility',
-    traceEventsToMeasure: [
-        'RenderAccessibilityImpl::SendPendingAccessibilityEvents',
-    ]
-});
-</script>
-
-</html>

third_party/blink/public/web/web_ax_object.h

@@ -390,12 +390,6 @@ class WebAXObject {
                                       SkMatrix44& container_transform,
                                       bool* clips_children = nullptr) const;
 
-  // Retrieves a vector of all WebAXObjects in this document whose
-  // bounding boxes may have changed since the last query. Can be called
-  // on any object.
-  BLINK_EXPORT void GetAllObjectsWithChangedBounds(
-      WebVector<WebAXObject>& out_changed_bounds_objects) const;
-
   // Blink-internal DOM Node ID. Currently used for PDF exporting.
   BLINK_EXPORT int GetDOMNodeId() const;
 

third_party/blink/renderer/core/accessibility/ax_object_cache.h

@@ -145,9 +145,6 @@ class CORE_EXPORT AXObjectCache : public GarbageCollected<AXObjectCache> {
   // without producing any layout or other notifications.
   virtual void HandleFrameRectsChanged(Document&) = 0;
 
-  // Called when a layout object's bounding box may have changed.
-  virtual void InvalidateBoundingBox(const LayoutObject*) = 0;
-
   virtual const AtomicString& ComputedRoleForNode(Node*) = 0;
   virtual String ComputedNameForNode(Node*) = 0;
 

third_party/blink/renderer/core/paint/paint_invalidator.cc

@@ -284,9 +284,6 @@ bool PaintInvalidator::InvalidatePaint(
         PaintInvalidatorContext::kSubtreeInvalidationChecking;
   }
 
-  if (AXObjectCache* cache = object.GetDocument().ExistingAXObjectCache())
-    cache->InvalidateBoundingBox(&object);
-
   return reason != PaintInvalidationReason::kNone;
 }
 

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

@@ -1999,18 +1999,6 @@ AXObject* AXObjectCacheImpl::GetActiveAriaModalDialog() const {
   return active_aria_modal_dialog_;
 }
 
-HeapVector<Member<AXObject>>
-AXObjectCacheImpl::GetAllObjectsWithChangedBounds() {
-  VectorOf<AXObject> changed_bounds_objects;
-  changed_bounds_objects.ReserveCapacity(changed_bounds_ids_.size());
-  for (AXID changed_bounds_id : changed_bounds_ids_) {
-    if (AXObject* obj = ObjectFromAXID(changed_bounds_id))
-      changed_bounds_objects.push_back(obj);
-  }
-  changed_bounds_ids_.clear();
-  return changed_bounds_objects;
-}
-
 void AXObjectCacheImpl::HandleInitialFocus() {
   PostNotification(document_, ax::mojom::Event::kFocus);
 }
@@ -2118,12 +2106,6 @@ void AXObjectCacheImpl::HandleFrameRectsChanged(Document& document) {
   MarkAXObjectDirty(Get(&document), false);
 }
 
-void AXObjectCacheImpl::InvalidateBoundingBox(
-    const LayoutObject* layout_object) {
-  if (AXObject* obj = Get(const_cast<LayoutObject*>(layout_object)))
-    changed_bounds_ids_.insert(obj->AXObjectID());
-}
-
 void AXObjectCacheImpl::HandleScrollPositionChanged(
     LocalFrameView* frame_view) {
   SCOPED_DISALLOW_LIFECYCLE_TRANSITION(*frame_view->GetFrame().GetDocument());

third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h

@@ -163,10 +163,6 @@ class MODULES_EXPORT AXObjectCacheImpl
   // without producing any layout or other notifications.
   void HandleFrameRectsChanged(Document&) override;
 
-  // Invalidates the bounding box, which can be later retrieved by
-  // GetAllObjectsWithChangedBounds.
-  void InvalidateBoundingBox(const LayoutObject*) override;
-
   const AtomicString& ComputedRoleForNode(Node*) override;
   String ComputedNameForNode(Node*) override;
 
@@ -286,11 +282,6 @@ class MODULES_EXPORT AXObjectCacheImpl
 
   AXObject* GetActiveAriaModalDialog() const;
 
-  // Retrieves a vector of all AXObjects whose bounding boxes may have changed
-  // since the last query. Clears the vector so that the next time it's
-  // called, it will only retrieve objects that have changed since now.
-  HeapVector<Member<AXObject>> GetAllObjectsWithChangedBounds();
-
  protected:
   void PostPlatformNotification(
       AXObject* obj,
@@ -499,10 +490,6 @@ class MODULES_EXPORT AXObjectCacheImpl
   // Maps ids to their object's autofill state.
   HashMap<AXID, WebAXAutofillState> autofill_state_map_;
 
-  // The set of node IDs whose bounds has changed since the last time
-  // GetAllObjectsWithChangedBounds was called.
-  HashSet<AXID> changed_bounds_ids_;
-
   // The source of the event that is currently being handled.
   ax::mojom::blink::EventFrom active_event_from_ =
       ax::mojom::blink::EventFrom::kNone;

third_party/blink/renderer/modules/exported/web_ax_object.cc

@@ -1502,20 +1502,6 @@ void WebAXObject::GetRelativeBounds(WebAXObject& offset_container,
   bounds_in_container = WebFloatRect(bounds);
 }
 
-void WebAXObject::GetAllObjectsWithChangedBounds(
-    WebVector<WebAXObject>& out_changed_bounds_objects) const {
-  if (IsDetached())
-    return;
-
-  HeapVector<Member<AXObject>> changed_bounds_objects =
-      private_->AXObjectCache().GetAllObjectsWithChangedBounds();
-
-  out_changed_bounds_objects.reserve(changed_bounds_objects.size());
-  out_changed_bounds_objects.resize(changed_bounds_objects.size());
-  std::copy(changed_bounds_objects.begin(), changed_bounds_objects.end(),
-            out_changed_bounds_objects.begin());
-}
-
 bool WebAXObject::ScrollToMakeVisible() const {
   if (IsDetached())
     return false;