Always try to send ax menuEnd

R=dmazzoni@chromium.org

Fixed: 1131321
Change-Id: If11517ade428937676330728733a6dccba0c0064
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560616
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831964}

chrome/browser/resources/chromeos/accessibility/chromevox/background/background_test.js

@@ -330,7 +330,10 @@ TEST_F('ChromeVoxBackgroundTest', 'ShowContextMenu', function() {
     mockFeedback.call(go.focus.bind(go))
         .expectSpeech('a', 'Link')
         .call(doCmd('contextMenu'))
-        .expectSpeech(/menu opened/);
+        .expectSpeech(/menu opened/)
+        .call(press(KeyCode.ESCAPE))
+        .expectSpeech(/menu closed/)
+        .expectSpeech('a', 'Link');
     mockFeedback.replay();
   }.bind(this));
 });

chrome/browser/ui/aura/accessibility/automation_manager_aura.cc

@@ -55,7 +55,7 @@ void AutomationManagerAura::Enable() {
 #endif
 
   // Send this event immediately to push the initial desktop tree state.
-  pending_events_.push_back({current_tree_->GetRoot()->GetUniqueId(),
+  pending_events_.push_back({tree_->GetRoot()->GetUniqueId(),
                              ax::mojom::Event::kLoadComplete, -1,
                              is_performing_action_});
   SendPendingEvents();
@@ -93,7 +93,7 @@ void AutomationManagerAura::OnViewEvent(views::View* view,
 }
 
 void AutomationManagerAura::HandleEvent(ax::mojom::Event event_type) {
-  views::AXAuraObjWrapper* obj = current_tree_->GetRoot();
+  views::AXAuraObjWrapper* obj = tree_->GetRoot();
   if (!obj)
     return;
 
@@ -123,7 +123,7 @@ void AutomationManagerAura::PerformAction(const ui::AXActionData& data) {
     return;
   }
 
-  current_tree_->HandleAccessibleAction(data);
+  tree_->HandleAccessibleAction(data);
 }
 
 void AutomationManagerAura::OnChildWindowRemoved(
@@ -132,7 +132,7 @@ void AutomationManagerAura::OnChildWindowRemoved(
     return;
 
   if (!parent)
-    parent = current_tree_->GetRoot();
+    parent = tree_->GetRoot();
 
   PostEvent(parent->GetUniqueId(), ax::mojom::Event::kChildrenChanged);
 }
@@ -151,18 +151,17 @@ AutomationManagerAura::AutomationManagerAura()
 AutomationManagerAura::~AutomationManagerAura() = default;
 
 void AutomationManagerAura::Reset(bool reset_serializer) {
-  if (!current_tree_) {
+  if (!tree_) {
     auto desktop_root = std::make_unique<AXRootObjWrapper>(this, cache_.get());
-    current_tree_ = std::make_unique<views::AXTreeSourceViews>(
+    tree_ = std::make_unique<views::AXTreeSourceViews>(
         desktop_root.get(), ax_tree_id(), cache_.get());
     cache_->CreateOrReplace(std::move(desktop_root));
   }
   if (reset_serializer) {
-    current_tree_serializer_.reset();
+    tree_serializer_.reset();
     alert_window_.reset();
   } else {
-    current_tree_serializer_ =
-        std::make_unique<AuraAXTreeSerializer>(current_tree_.get());
+    tree_serializer_ = std::make_unique<AuraAXTreeSerializer>(tree_.get());
 #if BUILDFLAG(IS_CHROMEOS_ASH)
     ash::Shell* shell = ash::Shell::Get();
     // Windows within the overlay container get moved to the new monitor when
@@ -195,7 +194,7 @@ void AutomationManagerAura::SendPendingEvents() {
   if (!enabled_)
     return;
 
-  if (!current_tree_serializer_)
+  if (!tree_serializer_)
     return;
 
   std::vector<ui::AXTreeUpdate> tree_updates;
@@ -206,11 +205,17 @@ void AutomationManagerAura::SendPendingEvents() {
     int id = event_copy.id;
     ax::mojom::Event event_type = event_copy.event_type;
     auto* aura_obj = cache_->Get(id);
+
+    // Some events are important enough where even if their ax obj was
+    // destroyed, they still need to be fired.
+    if (event_type == ax::mojom::Event::kMenuEnd && !aura_obj)
+      aura_obj = tree_->GetRoot();
+
     if (!aura_obj)
       continue;
 
     ui::AXTreeUpdate update;
-    if (!current_tree_serializer_->SerializeChanges(aura_obj, &update)) {
+    if (!tree_serializer_->SerializeChanges(aura_obj, &update)) {
       OnSerializeFailure(event_type, update);
       return;
     }
@@ -221,7 +226,7 @@ void AutomationManagerAura::SendPendingEvents() {
     // marked invisible, for example. In those cases we should still
     // call SerializeChanges (because the change may have affected the
     // ancestor) but we shouldn't fire the event on the node not in the tree.
-    if (current_tree_serializer_->IsInClientTree(aura_obj)) {
+    if (tree_serializer_->IsInClientTree(aura_obj)) {
       ui::AXEvent event;
       event.id = aura_obj->GetUniqueId();
       event.event_type = event_type;
@@ -236,7 +241,7 @@ void AutomationManagerAura::SendPendingEvents() {
   views::AXAuraObjWrapper* focus = cache_->GetFocus();
   if (focus) {
     ui::AXTreeUpdate focused_node_update;
-    current_tree_serializer_->SerializeChanges(focus, &focused_node_update);
+    tree_serializer_->SerializeChanges(focus, &focused_node_update);
     tree_updates.push_back(focused_node_update);
   }
 
@@ -316,7 +321,7 @@ void AutomationManagerAura::OnSerializeFailure(ax::mojom::Event event_type,
   std::string error_string;
   ui::AXTreeSourceChecker<views::AXAuraObjWrapper*, ui::AXNodeData,
                           ui::AXTreeData>
-      checker(current_tree_.get());
+      checker(tree_.get());
   checker.CheckAndGetErrorString(&error_string);
 
   // Add a crash key so we can figure out why this is happening.

chrome/browser/ui/aura/accessibility/automation_manager_aura.h

@@ -109,11 +109,11 @@ class AutomationManagerAura : public ui::AXActionHandler,
   // Holds the active views-based accessibility tree. A tree currently consists
   // of all views descendant to a |Widget| (see |AXTreeSourceViews|).
   // A tree becomes active when an event is fired on a descendant view.
-  std::unique_ptr<views::AXTreeSourceViews> current_tree_;
+  std::unique_ptr<views::AXTreeSourceViews> tree_;
 
   // Serializes incremental updates on the currently active tree
-  // |current_tree_|.
-  std::unique_ptr<AuraAXTreeSerializer> current_tree_serializer_;
+  // |tree_|.
+  std::unique_ptr<AuraAXTreeSerializer> tree_serializer_;
 
   bool processing_posted_ = false;
 

chrome/browser/ui/aura/accessibility/automation_manager_aura_browsertest.cc

@@ -158,7 +158,7 @@ IN_PROC_BROWSER_TEST_F(AutomationManagerAuraBrowserTest, WebAppearsOnce) {
 
   AutomationManagerAura* manager = AutomationManagerAura::GetInstance();
   manager->Enable();
-  auto* tree = manager->current_tree_.get();
+  auto* tree = manager->tree_.get();
 
   ui_test_utils::NavigateToURL(
       browser(),
@@ -255,7 +255,7 @@ IN_PROC_BROWSER_TEST_F(AutomationManagerAuraBrowserTest, ScrollView) {
   AutomationManagerAura* manager = AutomationManagerAura::GetInstance();
   manager->set_ax_aura_obj_cache_for_testing(std::move(cache));
   manager->Enable();
-  auto* tree = manager->current_tree_.get();
+  auto* tree = manager->tree_.get();
 
   // Create a widget with size 200, 200.
   views::Widget* widget = new views::Widget;
@@ -370,7 +370,7 @@ IN_PROC_BROWSER_TEST_F(AutomationManagerAuraBrowserTest, EventFromAction) {
   // accessibility event that shows this view is focused.
   ui::AXActionData action_data;
   action_data.action = ax::mojom::Action::kFocus;
-  action_data.target_tree_id = manager->current_tree_.get()->tree_id_for_test();
+  action_data.target_tree_id = manager->tree_.get()->tree_id_for_test();
   action_data.target_node_id = wrapper2->GetUniqueId();
 
   manager->PerformAction(action_data);