Use default action verb kClick instead of kClickable

Switching flutter bridge to use the default action verb kClick
rather than the boolean attribute kClickable.  The boolean
attribute was added specifically for Android and has unwanted
side-effects including not allowing nodes marked clickable
to have navigable descendants.  This CL removes a previous
work around that was caused by using clickable attribute.

Also changing the touch exploration controller to
always issue a simulated tap.  Ax gestures do not
always get propagated to the tap listener via the flutter
bridge.  This will make for a better user experience
for some ui elements that require simulated taps.

Bug: None
Test: Local display assistant build, unittests
Change-Id: I8bb9204d78ef61bb8afc727b9b6b41e2ca05390c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431557Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Commit-Queue: Randy Rossi <rmrossi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811732}

chromecast/browser/accessibility/flutter/ax_tree_source_flutter_unittest.cc

@@ -632,5 +632,27 @@ TEST_F(AXTreeSourceFlutterTest, ResetFocus) {
   ASSERT_EQ(0, tree_data.focus_id);
 }
 
+TEST_F(AXTreeSourceFlutterTest, NoClickable) {
+  OnAccessibilityEventRequest event;
+
+  event.set_source_id(0);
+  event.set_window_id(1);
+  event.set_event_type(OnAccessibilityEventRequest_EventType_FOCUSED);
+
+  SemanticsNode* root = event.add_node_data();
+  root->set_node_id(0);
+  ActionProperties* action_properties = root->mutable_action_properties();
+  action_properties->set_tap(true);
+  Rect* bounds = root->mutable_bounds_in_screen();
+  SetRect(bounds, 0, 0, 1280, 800);
+
+  CallNotifyAccessibilityEvent(&event);
+  std::unique_ptr<ui::AXNodeData> data;
+  CallSerializeNode(root, &data);
+
+  // No node should get the clickable attribute.
+  ASSERT_FALSE(data->GetBoolAttribute(ax::mojom::BoolAttribute::kClickable));
+}
+
 }  // namespace accessibility
 }  // namespace chromecast

chromecast/browser/accessibility/flutter/flutter_semantics_node_wrapper.cc

@@ -301,8 +301,9 @@ void FlutterSemanticsNodeWrapper::Serialize(ui::AXNodeData* out_data) const {
     out_data->SetValue(GetValue());
   }
 
-  out_data->AddBoolAttribute(ax::mojom::BoolAttribute::kClickable,
-                             IsActionable());
+  if (IsActionable()) {
+    out_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kClick);
+  }
   out_data->AddBoolAttribute(ax::mojom::BoolAttribute::kScrollable,
                              IsScrollable());
 
@@ -495,18 +496,8 @@ bool FlutterSemanticsNodeWrapper::HasTapOrPress() const {
 }
 
 bool FlutterSemanticsNodeWrapper::IsActionable() const {
-  // When flutter tells us a generic container is actionable, do not allow this
-  // unless all children of this node are NOT actionable.  Otherwise, the
-  // tree walker will consider this node a leaf and it will not navigate to
-  // any actionable children.
   bool actionable = HasTapOrPress();
 
-  ui::AXNodeData data;
-  PopulateAXRole(&data);
-  if (actionable && data.role == ax::mojom::Role::kGenericContainer &&
-      AnyChildIsActionable()) {
-    actionable = false;
-  }
   // If this node is actionable but is also the host for a child tree,
   // don't make it actionable or else chromevox won't traverse into
   // any child.

chromecast/browser/accessibility/touch_exploration_controller.cc

@@ -425,7 +425,7 @@ ui::EventDispatchDetails TouchExplorationController::InDoubleTapPending(
     if (current_touch_ids_.size() != 0)
       return DiscardEvent(continuation);
 
-    SendSimulatedClickOrTap(continuation);
+    SendSimulatedClick(continuation);
 
     SET_STATE(NO_FINGERS_DOWN);
     return DiscardEvent(continuation);
@@ -445,7 +445,7 @@ ui::EventDispatchDetails TouchExplorationController::InTouchReleasePending(
     if (current_touch_ids_.size() != 0)
       return DiscardEvent(continuation);
 
-    SendSimulatedClickOrTap(continuation);
+    SendSimulatedClick(continuation);
     SET_STATE(NO_FINGERS_DOWN);
     return DiscardEvent(continuation);
   }
@@ -590,7 +590,7 @@ ui::EventDispatchDetails TouchExplorationController::InTouchExploreSecondPress(
       return DiscardEvent(continuation);
     }
 
-    SendSimulatedClickOrTap(continuation);
+    SendSimulatedClick(continuation);
 
     SET_STATE(TOUCH_EXPLORATION);
     EnterTouchToMouseMode();
@@ -608,16 +608,13 @@ ui::EventDispatchDetails TouchExplorationController::InWaitForNoFingers(
   return DiscardEvent(continuation);
 }
 
-void TouchExplorationController::SendSimulatedClickOrTap(
+void TouchExplorationController::SendSimulatedClick(
     const Continuation continuation) {
-  // If we got an anchor point from ChromeVox, send a double-tap gesture
-  // and let ChromeVox handle the click.
   const gfx::Point location;
+  // For Chromecast, always send a simulated tap. NOTE: This differs
+  // from chromeos's touch exploration controller which always send an
+  // accessibility gesture.
   delegate_->HandleTap(location);
-  if (anchor_point_state_ == ANCHOR_POINT_EXPLICITLY_SET) {
-    delegate_->HandleAccessibilityGesture(ax::mojom::Gesture::kClick);
-    return;
-  }
   SendSimulatedTap(continuation);
 }
 

chromecast/browser/accessibility/touch_exploration_controller.h

@@ -265,9 +265,8 @@ class TouchExplorationController : public ui::EventRewriter,
 
   void PlaySoundForTimer();
 
-  // Sends a simulated click, if an anchor point was set explicitly. Otherwise,
-  // sends a simulated tap at anchor point.
-  void SendSimulatedClickOrTap(const Continuation continuation);
+  // Sends a simulated click.
+  void SendSimulatedClick(const Continuation continuation);
 
   // Sends a simulated tap at anchor point.
   void SendSimulatedTap(const Continuation continuation);

chromecast/browser/accessibility/touch_exploration_controller_unittest.cc

@@ -722,8 +722,9 @@ TEST_F(TouchExplorationTest, DoubleTapTiming) {
   generator_->ReleaseTouch();
 
   std::vector<ui::LocatedEvent*> captured_events = GetCapturedLocatedEvents();
-  ASSERT_EQ(0U, captured_events.size());
-  EXPECT_EQ(ax::mojom::Gesture::kClick, delegate_.GetLastGesture());
+  ASSERT_EQ(2U, captured_events.size());
+  EXPECT_EQ(ui::ET_TOUCH_PRESSED, captured_events[0]->type());
+  EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
 }
 
 // If an explicit anchor point is set during touch exploration, double-tapping
@@ -760,9 +761,10 @@ TEST_F(TouchExplorationTest, DoubleTapWithExplicitAnchorPoint) {
   generator_->ReleaseTouch();
 
   std::vector<ui::LocatedEvent*> captured_events = GetCapturedLocatedEvents();
-  ASSERT_EQ(0U, captured_events.size());
+  ASSERT_EQ(2U, captured_events.size());
   EXPECT_TRUE(IsInNoFingersDownState());
-  EXPECT_EQ(ax::mojom::Gesture::kClick, delegate_.GetLastGesture());
+  EXPECT_EQ(ui::ET_TOUCH_PRESSED, captured_events[0]->type());
+  EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
 }
 
 // Double-tapping where the user holds their finger down for the second time