Reland: Fire events on relation sources

Fixes performance regression by only sending event for non *created* tree changes.

 Bug: 808061
Original change's description:
> Add hooks for AXTreeDelegate when a relation target changes
>
>
> Problem
> Suppose a node a has an active descendant b. A page author might change |b| to aria-selected="true" immediately after setting |b| to be the active descendant.
> As a result, we have the events and actions:
> active descendant changed on |a|
> client gets event on |a| and emits output
> attribute changed on |b|
> client gets attribute changed on |b| but focus remains over |a|
>
> Solution
>
> Let the client know about changes in its related nodes.
> For example, changes to |b| would cause an event on |a|.
>
> Bug: 808061
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I58a3d99d3187db348320223f0bde588015ab6f55
> Reviewed-on: https://chromium-review.googlesource.com/899862
> Commit-Queue: David Tseng <dtseng@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534885}

TBR=dmazzoni@chromium.org,dtseng@chromium.org,nektar@chromium.org,aleventhal@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Iaf04a51602e091f9bb9767a6bb56e4a5177540d3
Reviewed-on: https://chromium-review.googlesource.com/924769Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537636}

chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs

@@ -1435,3 +1435,35 @@ TEST_F('BackgroundTest', 'TableColumnHeaders', function() {
         .replay();
   });
 });
+
+TEST_F('BackgroundTest', 'ActiveDescendantUpdates', function() {
+  var mockFeedback = this.createMockFeedback();
+  this.runWithLoadedTree(function(root) {/*!
+    <div aria-label="container" tabindex=0 role="group" id="active"
+        aria-activedescendant="1">
+      <div id="1" role="treeitem"></div>
+      <div id="2" role="treeitem"></div>
+    <script>
+      var alt = false;
+      var active = document.getElementById('active');
+      var one = document.getElementById('1');
+      var two = document.getElementById('2');
+      active.addEventListener('click', function() {
+        var sel = alt ? one : two;
+        var unsel = alt ? two : one;
+        active.setAttribute('aria-activedescendant', sel.id);
+        sel.setAttribute('aria-selected', true);
+        unsel.setAttribute('aria-selected', false);
+        alt = !alt;
+      });
+      </script>
+  */}, function(root) {
+    var group = root.firstChild;
+    mockFeedback.call(group.focus.bind(group))
+        .call(group.doDefault.bind(group))
+        .expectSpeech('Tree item', 'Selected', ' 2 of 2 ')
+        .call(group.doDefault.bind(group))
+        .expectSpeech('Tree item', 'Selected', ' 1 of 2 ')
+        .replay();
+  });
+});

chrome/browser/resources/chromeos/chromevox/cvox2/background/desktop_automation_handler.js

@@ -180,6 +180,11 @@ DesktopAutomationHandler.prototype = {
    * @param {!AutomationEvent} evt
    */
   onAriaAttributeChanged: function(evt) {
+    if (evt.target.activeDescendant) {
+      this.onActiveDescendantChanged(evt);
+      return;
+    }
+
     if (evt.target.state.editable)
       return;
     this.onEventIfInRange(evt);

chrome/renderer/extensions/automation_ax_tree_wrapper.cc

@@ -141,6 +141,8 @@ api::automation::EventType ToAutomationEvent(
       return api::automation::EVENT_TYPE_LOADCOMPLETE;
     case ui::AXEventGenerator::Event::MENU_ITEM_SELECTED:
       return api::automation::EVENT_TYPE_MENULISTITEMSELECTED;
+    case ui::AXEventGenerator::Event::RELATED_NODE_CHANGED:
+      return api::automation::EVENT_TYPE_ARIAATTRIBUTECHANGED;
     case ui::AXEventGenerator::Event::ROW_COUNT_CHANGED:
       return api::automation::EVENT_TYPE_ROWCOUNTCHANGED;
     case ui::AXEventGenerator::Event::SCROLL_POSITION_CHANGED:
@@ -150,7 +152,8 @@ api::automation::EventType ToAutomationEvent(
     case ui::AXEventGenerator::Event::VALUE_CHANGED:
       return api::automation::EVENT_TYPE_VALUECHANGED;
 
-    // These don't have a mapping.
+    // Map these into generic attribute changes (not necessarily aria related,
+    // but mapping for backward compat).
     case ui::AXEventGenerator::Event::COLLAPSED:
     case ui::AXEventGenerator::Event::DESCRIPTION_CHANGED:
     case ui::AXEventGenerator::Event::DOCUMENT_TITLE_CHANGED:
@@ -161,7 +164,7 @@ api::automation::EventType ToAutomationEvent(
     case ui::AXEventGenerator::Event::ROLE_CHANGED:
     case ui::AXEventGenerator::Event::SELECTED_CHANGED:
     case ui::AXEventGenerator::Event::STATE_CHANGED:
-      return api::automation::EVENT_TYPE_NONE;
+      return api::automation::EVENT_TYPE_ARIAATTRIBUTECHANGED;
   }
 
   NOTREACHED();
@@ -280,6 +283,7 @@ bool AutomationAXTreeWrapper::IsEventTypeHandledByAXEventGenerator(
   switch (event_type) {
     // Generated by AXEventGenerator.
     case api::automation::EVENT_TYPE_ACTIVEDESCENDANTCHANGED:
+    case api::automation::EVENT_TYPE_ARIAATTRIBUTECHANGED:
     case api::automation::EVENT_TYPE_CHECKEDSTATECHANGED:
     case api::automation::EVENT_TYPE_EXPANDEDCHANGED:
     case api::automation::EVENT_TYPE_INVALIDSTATUSCHANGED:
@@ -322,7 +326,6 @@ bool AutomationAXTreeWrapper::IsEventTypeHandledByAXEventGenerator(
 
     // These events might need to be migrated to AXEventGenerator.
     case api::automation::EVENT_TYPE_ALERT:
-    case api::automation::EVENT_TYPE_ARIAATTRIBUTECHANGED:
     case api::automation::EVENT_TYPE_BLUR:
     case api::automation::EVENT_TYPE_CHILDRENCHANGED:
     case api::automation::EVENT_TYPE_DOCUMENTSELECTIONCHANGED:

content/browser/accessibility/browser_accessibility_manager_android.cc

@@ -208,6 +208,7 @@ void BrowserAccessibilityManagerAndroid::FireGeneratedEvent(
     case Event::MENU_ITEM_SELECTED:
     case Event::NAME_CHANGED:
     case Event::OTHER_ATTRIBUTE_CHANGED:
+    case Event::RELATED_NODE_CHANGED:
     case Event::ROLE_CHANGED:
     case Event::ROW_COUNT_CHANGED:
     case Event::SELECTED_CHANGED:

content/browser/accessibility/browser_accessibility_manager_mac.mm

@@ -360,6 +360,7 @@ void BrowserAccessibilityManagerMac::FireGeneratedEvent(
     case Event::LIVE_REGION_NODE_CHANGED:
     case Event::NAME_CHANGED:
     case Event::OTHER_ATTRIBUTE_CHANGED:
+    case Event::RELATED_NODE_CHANGED:
     case Event::ROLE_CHANGED:
     case Event::SCROLL_POSITION_CHANGED:
     case Event::SELECTED_CHANGED:

content/browser/accessibility/browser_accessibility_manager_win.cc

@@ -181,6 +181,7 @@ void BrowserAccessibilityManagerWin::FireGeneratedEvent(
     case Event::MENU_ITEM_SELECTED:
     case Event::NAME_CHANGED:
     case Event::OTHER_ATTRIBUTE_CHANGED:
+    case Event::RELATED_NODE_CHANGED:
     case Event::ROLE_CHANGED:
     case Event::ROW_COUNT_CHANGED:
     case Event::SELECTED_CHANGED:

ui/accessibility/ax_event_generator.cc

@@ -128,6 +128,9 @@ void AXEventGenerator::OnStateChanged(AXTree* tree,
   AddEvent(node, Event::STATE_CHANGED);
   if (state == ax::mojom::State::kExpanded) {
     AddEvent(node, new_value ? Event::EXPANDED : Event::COLLAPSED);
+
+    // TODO(accessibility): tree in the midst of updates. Disallow access to
+    // |node|.
     if (node->data().role == ax::mojom::Role::kRow ||
         node->data().role == ax::mojom::Role::kTreeItem) {
       ui::AXNode* container = node;
@@ -158,6 +161,9 @@ void AXEventGenerator::OnStringAttributeChanged(AXTree* tree,
   switch (attr) {
     case ax::mojom::StringAttribute::kName:
       AddEvent(node, Event::NAME_CHANGED);
+
+      // TODO(accessibility): tree in the midst of updates. Disallow
+      // access to |node|.
       if (node->data().HasStringAttribute(
               ax::mojom::StringAttribute::kContainerLiveStatus)) {
         FireLiveRegionEvents(node);
@@ -173,6 +179,8 @@ void AXEventGenerator::OnStringAttributeChanged(AXTree* tree,
       AddEvent(node, Event::INVALID_STATUS_CHANGED);
       break;
     case ax::mojom::StringAttribute::kLiveStatus:
+      // TODO(accessibility): tree in the midst of updates. Disallow access to
+      // |node|.
       if (node->data().role != ax::mojom::Role::kAlert)
         AddEvent(node, Event::LIVE_REGION_CREATED);
       break;
@@ -328,6 +336,9 @@ void AXEventGenerator::OnAtomicUpdateFinished(
         FireLiveRegionEvents(change.node);
       }
     }
+
+    if (change.type != NODE_CREATED && change.type != SUBTREE_CREATED)
+      FireRelationSourceEvents(tree, change.node);
   }
 
   FireActiveDescendantEvents();
@@ -371,4 +382,32 @@ void AXEventGenerator::FireActiveDescendantEvents() {
   active_descendant_changed_.clear();
 }
 
+void AXEventGenerator::FireRelationSourceEvents(AXTree* tree,
+                                                AXNode* target_node) {
+  int32_t target_id = target_node->id();
+  std::set<AXNode*> source_nodes;
+  auto callback = [&](const auto entry) {
+    const auto target_to_sources = entry.second;
+    auto sources_it = target_to_sources.find(target_id);
+    if (sources_it == target_to_sources.end())
+      return;
+
+    auto sources = sources_it->second;
+    std::for_each(sources.begin(), sources.end(), [&](int32_t source_id) {
+      AXNode* source_node = tree->GetFromId(source_id);
+
+      if (!source_node || source_nodes.count(source_node) > 0)
+        return;
+
+      source_nodes.insert(source_node);
+      AddEvent(source_node, Event::RELATED_NODE_CHANGED);
+    });
+  };
+
+  std::for_each(tree->int_reverse_relations().begin(),
+                tree->int_reverse_relations().end(), callback);
+  std::for_each(tree->intlist_reverse_relations().begin(),
+                tree->intlist_reverse_relations().end(), callback);
+}
+
 }  // namespace ui

ui/accessibility/ax_event_generator.h

@@ -38,6 +38,7 @@ class AX_EXPORT AXEventGenerator : public AXTreeDelegate {
     MENU_ITEM_SELECTED,
     NAME_CHANGED,
     OTHER_ATTRIBUTE_CHANGED,
+    RELATED_NODE_CHANGED,
     ROLE_CHANGED,
     ROW_COUNT_CHANGED,
     SCROLL_POSITION_CHANGED,
@@ -169,6 +170,7 @@ class AX_EXPORT AXEventGenerator : public AXTreeDelegate {
  private:
   void FireLiveRegionEvents(AXNode* node);
   void FireActiveDescendantEvents();
+  void FireRelationSourceEvents(AXTree* tree, AXNode* target_node);
 
   AXTree* tree_ = nullptr;  // Not owned.
   std::map<AXNode*, std::set<Event>> tree_events_;

ui/accessibility/ax_event_generator_unittest.cc

@@ -71,6 +71,9 @@ std::string DumpEvents(AXEventGenerator* generator) {
       case AXEventGenerator::Event::OTHER_ATTRIBUTE_CHANGED:
         event_name = "OTHER_ATTRIBUTE_CHANGED";
         break;
+      case AXEventGenerator::Event::RELATED_NODE_CHANGED:
+        event_name = "RELATED_NODE_CHANGED";
+        break;
       case AXEventGenerator::Event::ROLE_CHANGED:
         event_name = "ROLE_CHANGED";
         break;
@@ -349,7 +352,10 @@ TEST(AXEventGeneratorTest, ActiveDescendantChanged) {
   update.nodes[0].AddIntAttribute(ax::mojom::IntAttribute::kActivedescendantId,
                                   3);
   EXPECT_TRUE(tree.Unserialize(update));
-  EXPECT_EQ("ACTIVE_DESCENDANT_CHANGED on 1", DumpEvents(&event_generator));
+  EXPECT_EQ(
+      "ACTIVE_DESCENDANT_CHANGED on 1, "
+      "RELATED_NODE_CHANGED on 1",
+      DumpEvents(&event_generator));
 }
 
 TEST(AXEventGeneratorTest, CreateAlertAndLiveRegion) {
@@ -625,7 +631,8 @@ TEST(AXEventGeneratorTest, OtherAttributeChanged) {
       "OTHER_ATTRIBUTE_CHANGED on 3, "
       "OTHER_ATTRIBUTE_CHANGED on 4, "
       "OTHER_ATTRIBUTE_CHANGED on 5, "
-      "OTHER_ATTRIBUTE_CHANGED on 6",
+      "OTHER_ATTRIBUTE_CHANGED on 6, "
+      "RELATED_NODE_CHANGED on 6",
       DumpEvents(&event_generator));
 }
 
@@ -697,7 +704,8 @@ TEST(AXEventGeneratorTest, MenuItemSelected) {
   EXPECT_TRUE(tree.Unserialize(update));
   EXPECT_EQ(
       "ACTIVE_DESCENDANT_CHANGED on 1, "
-      "MENU_ITEM_SELECTED on 3",
+      "MENU_ITEM_SELECTED on 3, "
+      "RELATED_NODE_CHANGED on 1",
       DumpEvents(&event_generator));
 }
 

ui/accessibility/ax_tree.cc

@@ -464,6 +464,10 @@ bool AXTree::UpdateNode(const AXNodeData& src,
   AXNode* node = GetFromId(src.id);
   if (node) {
     update_state->pending_nodes.erase(node);
+
+    // TODO(accessibility): CallNodeChangeCallbacks should not pass |node|,
+    // since the tree and the node data are not yet in a consistent
+    // state. Possibly only pass id.
     if (update_state->new_nodes.find(node) == update_state->new_nodes.end())
       CallNodeChangeCallbacks(node, src);
     UpdateReverseRelations(node, src);

ui/accessibility/ax_tree.h

@@ -158,6 +158,13 @@ class AX_EXPORT AXTreeDelegate {
 // accessibility APIs on a specific platform.
 class AX_EXPORT AXTree {
  public:
+  typedef std::map<ax::mojom::IntAttribute,
+                   std::map<int32_t, std::set<int32_t>>>
+      IntReverseRelationMap;
+  typedef std::map<ax::mojom::IntListAttribute,
+                   std::map<int32_t, std::set<int32_t>>>
+      IntListReverseRelationMap;
+
   AXTree();
   explicit AXTree(const AXTreeUpdate& initial_state);
   virtual ~AXTree();
@@ -214,6 +221,13 @@ class AX_EXPORT AXTree {
   std::set<int32_t> GetReverseRelations(ax::mojom::IntListAttribute attr,
                                         int32_t dst_id) const;
 
+  // Map from a relation attribute to a map from a target id to source ids.
+  const IntReverseRelationMap& int_reverse_relations() {
+    return int_reverse_relations_;
+  }
+  const IntListReverseRelationMap& intlist_reverse_relations() {
+    return intlist_reverse_relations_;
+  }
   // Return a multi-line indented string representation, for logging.
   std::string ToString() const;
 
@@ -273,12 +287,10 @@ class AX_EXPORT AXTree {
 
   // Map from an int attribute (if IsNodeIdIntAttribute is true) to
   // a reverse mapping from target nodes to source nodes.
-  std::map<ax::mojom::IntAttribute, std::map<int32_t, std::set<int32_t>>>
-      int_reverse_relations_;
+  IntReverseRelationMap int_reverse_relations_;
   // Map from an int list attribute (if IsNodeIdIntListAttribute is true) to
   // a reverse mapping from target nodes to source nodes.
-  std::map<ax::mojom::IntListAttribute, std::map<int32_t, std::set<int32_t>>>
-      intlist_reverse_relations_;
+  IntListReverseRelationMap intlist_reverse_relations_;
 };
 
 }  // namespace ui