Separate sending AX events from updating dirty nodes, remove duplicates.

Splits code that loops over accessibility events from the code that
loops over dirty nodes that need to be serialized. Keeps track of
duplicate ids and avoids serializing the same node twice.

This is a different approach than crrev.com/c/1063007
"Avoid serializing the same accessibility node twice in the
same message" which tried to put all of the nodes in
the same AXTreeUpdate. That approach could fail in rare cases when
multiple updates both tried to set node_id_to_clear.
Now we've refactored the event message so that it can
contain multiple AXTreeUpdates, so it's safe to just skip
an entire update if it's a node we've already serialized
in this event bundle.

Bug: 651614, 845778

Change-Id: I3cc859e7a63d9c8b8beeb930d96aa53898940485
Reviewed-on: https://chromium-review.googlesource.com/1097423
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568195}

content/renderer/accessibility/render_accessibility_impl.cc

@@ -417,9 +417,14 @@ void RenderAccessibilityImpl::SendPendingAccessibilityEvents() {
   // The serialized event bundle to send to the browser.
   AccessibilityHostMsg_EventBundleParams bundle;
 
+  // Keep track of nodes in the tree that need to be updated.
+  std::vector<DirtyObject> dirty_objects;
+
   // If there's a layout complete message, we need to send location changes.
   bool had_layout_complete_messages = false;
 
+  ScopedFreezeBlinkAXTreeSource freeze(&tree_source_);
+
   // Loop over each event and generate an updated event message.
   for (size_t i = 0; i < src_events.size(); ++i) {
     ui::AXEvent& event = src_events[i];
@@ -436,8 +441,6 @@ void RenderAccessibilityImpl::SendPendingAccessibilityEvents() {
     while (!obj.IsDetached() && obj.AccessibilityIsIgnored())
       obj = obj.ParentObject();
 
-    ScopedFreezeBlinkAXTreeSource freeze(&tree_source_);
-
     // Make sure it's a descendant of our root node - exceptions include the
     // scroll area that's the parent of the main document (we ignore it), and
     // possibly nodes attached to a different document.
@@ -449,8 +452,22 @@ void RenderAccessibilityImpl::SendPendingAccessibilityEvents() {
     VLOG(1) << "Accessibility event: " << ui::ToString(event.event_type)
             << " on node id " << event.id;
 
+    DirtyObject dirty_object;
+    dirty_object.obj = obj;
+    dirty_object.event_from = event.event_from;
+    dirty_objects.push_back(dirty_object);
+  }
+
+  // Now serialize all dirty objects. Keep track of IDs serialized
+  // so we don't have to serialize the same node twice.
+  std::set<int32_t> already_serialized_ids;
+  for (size_t i = 0; i < dirty_objects.size(); i++) {
+    auto obj = dirty_objects[i].obj;
+    if (already_serialized_ids.find(obj.AxID()) != already_serialized_ids.end())
+      continue;
+
     AXContentTreeUpdate update;
-    update.event_from = event.event_from;
+    update.event_from = dirty_objects[i].event_from;
     // If there's a plugin, force the tree data to be generated in every
     // message so the plugin can merge its own tree data changes.
     if (plugin_tree_source_)
@@ -475,6 +492,10 @@ void RenderAccessibilityImpl::SendPendingAccessibilityEvents() {
       if (src.transform)
         dst.transform.reset(new gfx::Transform(*src.transform));
     }
+
+    for (size_t j = 0; j < update.nodes.size(); ++j)
+      already_serialized_ids.insert(update.nodes[j].id);
+
     bundle.updates.push_back(update);
 
     VLOG(1) << "Accessibility tree update:\n" << update.ToString();
@@ -494,7 +515,6 @@ void RenderAccessibilityImpl::SendLocationChanges() {
   std::vector<AccessibilityHostMsg_LocationChangeParams> messages;
 
   // Update layout on the root of the tree.
-  ScopedFreezeBlinkAXTreeSource freeze(&tree_source_);
   WebAXObject root = tree_source_.GetRoot();
   if (!root.UpdateLayoutAndCheckValidity())
     return;

content/renderer/accessibility/render_accessibility_impl.h

@@ -114,6 +114,11 @@ class CONTENT_EXPORT RenderAccessibilityImpl
   void SendLocationChanges();
 
  private:
+  struct DirtyObject {
+    blink::WebAXObject obj;
+    ax::mojom::EventFrom event_from;
+  };
+
   // RenderFrameObserver implementation.
   void OnDestruct() override;