[fucshia] Remove pending_accessibility_action_callbacks_ map

* Handle OnAccessibilityActionRequestedCallback once action has been
  dispatched
* Remove map because guarantees aren't provided for if actions will
  return.

Test: AccessibilityBridgeTest.PerformDefaultAction
Bug: 1111141
Change-Id: Ib8242be381f0336d312b9e205abe173636cf956e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328052Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Commit-Queue: Sharon Yang <yangsharon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795251}

fuchsia/engine/browser/accessibility_bridge.cc

@@ -26,20 +26,20 @@ AccessibilityBridge::AccessibilityBridge(
     fuchsia::accessibility::semantics::SemanticsManagerPtr semantics_manager,
     fuchsia::ui::views::ViewRef view_ref,
     content::WebContents* web_contents,
-    base::OnceCallback<void(zx_status_t)> on_disconnect_callback)
-    : binding_(this), web_contents_(web_contents) {
+    base::OnceCallback<void(zx_status_t)> on_error_callback)
+    : binding_(this),
+      web_contents_(web_contents),
+      on_error_callback_(std::move(on_error_callback)) {
   DCHECK(web_contents_);
   Observe(web_contents_);
   tree_.AddObserver(this);
 
   semantics_manager->RegisterViewForSemantics(
       std::move(view_ref), binding_.NewBinding(), tree_ptr_.NewRequest());
-  tree_ptr_.set_error_handler(
-      [disconnect_callback =
-           std::move(on_disconnect_callback)](zx_status_t status) mutable {
-        ZX_LOG(ERROR, status) << "SemanticTree disconnected";
-        std::move(disconnect_callback).Run(ZX_ERR_INTERNAL);
-      });
+  tree_ptr_.set_error_handler([this](zx_status_t status) mutable {
+    ZX_LOG(ERROR, status) << "SemanticTree disconnected";
+    std::move(on_error_callback_).Run(ZX_ERR_INTERNAL);
+  });
 }
 
 AccessibilityBridge::~AccessibilityBridge() {
@@ -50,11 +50,6 @@ AccessibilityBridge::~AccessibilityBridge() {
     hit.set_node_id(kSemanticNodeRootId);
     callback.second(std::move(hit));
   }
-  pending_hit_test_callbacks_.clear();
-
-  for (auto& callback : pending_accessibility_action_callbacks_)
-    callback.second(false);
-  pending_accessibility_action_callbacks_.clear();
 }
 
 void AccessibilityBridge::TryCommit() {
@@ -131,7 +126,12 @@ void AccessibilityBridge::AccessibilityEventReceived(
     const content::AXEventNotificationDetails& details) {
   // Updates to AXTree must be applied first.
   for (const ui::AXTreeUpdate& update : details.updates) {
-    tree_.Unserialize(update);
+    if (!tree_.Unserialize(update)) {
+      // If this fails, it is a fatal error that will cause the current Frame to
+      // be destroyed.
+      std::move(on_error_callback_).Run(ZX_ERR_INTERNAL);
+      return;
+    }
   }
 
   // Events to fire after tree has been updated.
@@ -147,13 +147,6 @@ void AccessibilityBridge::AccessibilityEventReceived(
         pending_hit_test_callbacks_.erase(event.action_request_id);
       }
     }
-
-    // Run callbacks associated with other accessibility events.
-    if (pending_accessibility_action_callbacks_.find(event.id) !=
-        pending_accessibility_action_callbacks_.end()) {
-      pending_accessibility_action_callbacks_[event.id](true);
-      pending_accessibility_action_callbacks_.erase(event.id);
-    }
   }
 }
 
@@ -169,15 +162,9 @@ void AccessibilityBridge::OnAccessibilityActionRequested(
   }
 
   action_data.target_node_id = node_id;
-  pending_accessibility_action_callbacks_[node_id] = std::move(callback);
-
-  // Allow tests to bypass action handling to simulate the case that actions
-  // aren't handled in tests.
-  if (!handle_actions_for_test_) {
-    return;
-  }
 
   web_contents_->GetMainFrame()->AccessibilityPerformAction(action_data);
+  callback(true);
 }
 
 void AccessibilityBridge::HitTest(fuchsia::math::PointF local_point,

fuchsia/engine/browser/accessibility_bridge.h

@@ -43,16 +43,12 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
       fuchsia::accessibility::semantics::SemanticsManagerPtr semantics_manager,
       fuchsia::ui::views::ViewRef view_ref,
       content::WebContents* web_contents,
-      base::OnceCallback<void(zx_status_t)> on_disconnect_callback);
+      base::OnceCallback<void(zx_status_t)> on_error_callback);
   ~AccessibilityBridge() final;
 
   AccessibilityBridge(const AccessibilityBridge&) = delete;
   AccessibilityBridge& operator=(const AccessibilityBridge&) = delete;
 
-  void set_handle_actions_for_test(bool handle) {
-    handle_actions_for_test_ = handle;
-  }
-
  private:
   FRIEND_TEST_ALL_PREFIXES(AccessibilityBridgeTest, OnSemanticsModeChanged);
 
@@ -127,15 +123,12 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   // These are keyed by the request_id field of ui::AXActionData.
   base::flat_map<int, HitTestCallback> pending_hit_test_callbacks_;
 
-  // Maintain a map of callbacks for accessibility actions. Entries are keyed by
-  // node id the action is performed on.
-  base::flat_map<int, OnAccessibilityActionRequestedCallback>
-      pending_accessibility_action_callbacks_;
+  // Run in the case of an internal error that cannot be recovered from. This
+  // will cause the frame |this| is owned by to be torn down.
+  base::OnceCallback<void(zx_status_t)> on_error_callback_;
 
   // The root id of |tree_|.
   int32_t root_id_ = 0;
-
-  bool handle_actions_for_test_ = true;
 };
 
 #endif  // FUCHSIA_ENGINE_BROWSER_ACCESSIBILITY_BRIDGE_H_

fuchsia/engine/browser/accessibility_bridge_browsertest.cc

@@ -231,16 +231,6 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformDefaultAction) {
   semantics_manager_.RequestAccessibilityAction(
       button2->node_id(), fuchsia::accessibility::semantics::Action::DEFAULT);
   semantics_manager_.RunUntilNumActionsHandledEquals(2);
-
-  // Handle the case that actions are still in flight when AccessibilityBridge
-  // gets torn down. The corresponding callbacks should still be run.
-  frame_impl_->set_handle_actions_for_test(false);
-  semantics_manager_.RequestAccessibilityAction(
-      button3->node_id(), fuchsia::accessibility::semantics::Action::DEFAULT);
-  frame_ptr_.Unbind();
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(2, semantics_manager_.num_actions_handled());
-  EXPECT_EQ(1, semantics_manager_.num_actions_unhandled());
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformUnsupportedAction) {

fuchsia/engine/browser/frame_impl.h

@@ -83,9 +83,6 @@ class FrameImpl : public fuchsia::web::Frame,
           semantics_manager) {
     semantics_manager_for_test_ = std::move(semantics_manager);
   }
-  void set_handle_actions_for_test(bool handle) {
-    accessibility_bridge_->set_handle_actions_for_test(handle);
-  }
   CastStreamingSessionClient* cast_streaming_session_client_for_test() {
     return cast_streaming_session_client_.get();
   }