Reland "[fuchsia][a11y] Sends full semantic tree when semantics are enabled."

Reland explanation: A test I wrote tested specific size values for the semantic
tree. The chrome a11y code that creates the tree for us is non-deterministic, so
we can't check for a specific tree size. I updated the tests to only test for a
minimum threshold, which is fine in this case here.



This is a reland of fdae2298

Original change's description:
> [fuchsia][a11y] Sends full semantic tree when semantics are enabled.
>
> This change fixes the bug where chrome was sending the full semantic tree only for the first time that semantics were enabled. This happened because the semantic tree was not getting cleared on the chrome side when disabled. when semantic were disabled / enabled, the events fired on the tree included only the diffs from the previous state of the tree.
>
> Test: web_engine_browsertests
>
> Bug: fuchsia:59680, 1134147
> Change-Id: I8266d6436f9bfb9ee9e89a0f46c0916bac7a5e75
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2442646
> Commit-Queue: Lucas Radaelli <lucasradaelli@google.com>
> Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
> Reviewed-by: Sharon Yang <yangsharon@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#813771}

Bug: fuchsia:59680
Bug: 1134147
Change-Id: I606b7628c480b775c04d84894faaba73f74789c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2450704
Commit-Queue: Lucas Radaelli <lucasradaelli@google.com>
Auto-Submit: Lucas Radaelli <lucasradaelli@google.com>
Reviewed-by: Sharon Yang <yangsharon@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813962}

fuchsia/engine/browser/accessibility_bridge.cc

@@ -47,13 +47,7 @@ AccessibilityBridge::AccessibilityBridge(
 }
 
 AccessibilityBridge::~AccessibilityBridge() {
-  // Acknowledge to the SemanticsManager if any actions have not been handled
-  // upon destruction time.
-  for (auto& callback : pending_hit_test_callbacks_) {
-    fuchsia::accessibility::semantics::Hit hit;
-    hit.set_node_id(kSemanticNodeRootId);
-    callback.second(std::move(hit));
-  }
+  InterruptPendingActions();
 }
 
 void AccessibilityBridge::TryCommit() {
@@ -117,6 +111,11 @@ void AccessibilityBridge::OnCommitComplete() {
 
 void AccessibilityBridge::AccessibilityEventReceived(
     const content::AXEventNotificationDetails& details) {
+  if (!enable_semantic_updates_) {
+    // No need to process events if Fuchsia is not receiving them.
+    return;
+  }
+
   // Updates to AXTree must be applied first.
   for (const ui::AXTreeUpdate& update : details.updates) {
     if (!ax_tree_.Unserialize(update)) {
@@ -201,18 +200,29 @@ void AccessibilityBridge::HitTest(fuchsia::math::PointF local_point,
 void AccessibilityBridge::OnSemanticsModeChanged(
     bool updates_enabled,
     OnSemanticsModeChangedCallback callback) {
+  // TODO(https://crbug.com/1134591): Enabling / disabling semantics can lead to
+  // race conditions.
+  if (enable_semantic_updates_ == updates_enabled)
+    return callback();
+
+  enable_semantic_updates_ = updates_enabled;
   if (updates_enabled) {
-    // The first call to AccessibilityEventReceived after this call will be the
-    // entire semantic tree.
+    // The first call to AccessibilityEventReceived after this call will be
+    // the entire semantic tree.
     web_contents_->EnableWebContentsOnlyAccessibilityMode();
   } else {
-    // The SemanticsManager will clear all state in this case, which is mirrored
-    // here.
+    // The SemanticsManager will clear all state in this case, which is
+    // mirrored here.
+    ui::AXMode mode = web_contents_->GetAccessibilityMode();
+    mode.set_mode(ui::AXMode::kWebContents, false);
+    web_contents_->SetAccessibilityMode(mode);
     to_send_.clear();
     commit_inflight_ = false;
+    ax_tree_.Destroy();
+    InterruptPendingActions();
   }
 
-  // Notify the SemanticsManager that semantics mode has been updated.
+  // Notify the SemanticsManager that this request was handled.
   callback();
 }
 
@@ -269,3 +279,13 @@ void AccessibilityBridge::OnAtomicUpdateFinished(
   }
   TryCommit();
 }
+
+void AccessibilityBridge::InterruptPendingActions() {
+  // Acknowledge to the SemanticsManager if any actions have not been handled
+  // upon destruction time or when semantic updates have been disabled.
+  for (auto& callback : pending_hit_test_callbacks_) {
+    fuchsia::accessibility::semantics::Hit hit;
+    callback.second(std::move(hit));
+  }
+  pending_hit_test_callbacks_.clear();
+}

fuchsia/engine/browser/accessibility_bridge.h

@@ -91,6 +91,10 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   // accessibility bridge.
   void DeleteSubtree(ui::AXNode* node);
 
+  // Interrupts actions that are waiting for a response. This is invoked during
+  // destruction time or when semantic updates have been disabled.
+  void InterruptPendingActions();
+
   // content::WebContentsObserver implementation.
   void AccessibilityEventReceived(
       const content::AXEventNotificationDetails& details) override;
@@ -118,6 +122,9 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   content::WebContents* web_contents_;
   ui::AXSerializableTree ax_tree_;
 
+  // Whether semantic updates are enabled.
+  bool enable_semantic_updates_ = false;
+
   // Cache for pending data to be sent to the Semantic Tree between commits.
   std::vector<SemanticUpdateOrDelete> to_send_;
   bool commit_inflight_ = false;

fuchsia/engine/browser/accessibility_bridge_browsertest.cc

@@ -31,7 +31,7 @@ const char kButtonName3[] = "button 3";
 const char kNodeName[] = "last node";
 const char kParagraphName[] = "a third paragraph";
 const char kOffscreenNodeName[] = "offscreen node";
-const size_t kPage1NodeCount = 9;
+const size_t kPage1NodeCount = 29;
 const size_t kPage2NodeCount = 190;
 const size_t kInitialRangeValue = 51;
 const size_t kStepSize = 3;
@@ -81,6 +81,14 @@ class AccessibilityBridgeTest : public cr_fuchsia::WebEngineBrowserTest {
     semantics_manager_.SetSemanticsModeEnabled(true);
   }
 
+  void LoadPage(base::StringPiece url, base::StringPiece page_title) {
+    GURL page_url(embedded_test_server()->GetURL(std::string(url)));
+    ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
+        navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
+        page_url.spec()));
+    navigation_listener_.RunUntilUrlAndTitleEquals(page_url, page_title);
+  }
+
  protected:
   fuchsia::web::FramePtr frame_ptr_;
   FrameImpl* frame_impl_;
@@ -106,11 +114,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, RegisterViewRef) {
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, CorrectDataSent) {
-  GURL page_url(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
 
   // Check that the data values are correct in the FakeSemanticTree.
   // TODO(fxb/18796): Test more fields once Chrome to Fuchsia conversions are
@@ -128,11 +132,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, CorrectDataSent) {
 // maximum, as set on the Fuchsia side. Check that all nodes are received by the
 // Semantic Tree when batching is performed.
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, DataSentWithBatching) {
-  GURL page_url(embedded_test_server()->GetURL(kPage2Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage2Title);
+  LoadPage(kPage2Path, kPage2Title);
 
   // Run until we expect more than a batch's worth of nodes to be present.
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage2NodeCount);
@@ -142,11 +142,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, DataSentWithBatching) {
 // Check that semantics information is correctly sent when navigating from page
 // to page.
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, TestNavigation) {
-  GURL page_url1(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url1.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url1, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
 
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
   EXPECT_TRUE(
@@ -156,10 +152,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, TestNavigation) {
   EXPECT_TRUE(
       semantics_manager_.semantic_tree()->GetNodeFromLabel(kParagraphName));
 
-  GURL page_url2(embedded_test_server()->GetURL(kPage2Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url2.spec()));
+  LoadPage(kPage2Path, kPage2Title);
 
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage2NodeCount);
   EXPECT_TRUE(
@@ -176,11 +169,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, TestNavigation) {
 // Checks that the correct node ID is returned when performing hit testing.
 // TODO(https://crbug.com/1050049): Re-enable once flake is fixed.
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, DISABLED_HitTest) {
-  GURL page_url(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
 
   fuchsia::accessibility::semantics::Node* hit_test_node =
       semantics_manager_.semantic_tree()->GetNodeFromLabel(kParagraphName);
@@ -203,11 +192,8 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, DISABLED_HitTest) {
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformDefaultAction) {
-  GURL page_url(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
+
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
 
   fuchsia::accessibility::semantics::Node* button1 =
@@ -229,11 +215,8 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformDefaultAction) {
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformUnsupportedAction) {
-  GURL page_url(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
+
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
 
   fuchsia::accessibility::semantics::Node* button1 =
@@ -271,11 +254,8 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformScrollToMakeVisible) {
   constexpr int kScreenHeight = 640;
   gfx::Rect screen_bounds(kScreenWidth, kScreenHeight);
 
-  GURL page_url(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
+
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
 
   auto* content_view =
@@ -311,11 +291,8 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, PerformScrollToMakeVisible) {
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, Slider) {
-  GURL page_url(embedded_test_server()->GetURL(kPage1Path));
-  ASSERT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
-      navigation_controller_.get(), fuchsia::web::LoadUrlParams(),
-      page_url.spec()));
-  navigation_listener_.RunUntilUrlAndTitleEquals(page_url, kPage1Title);
+  LoadPage(kPage1Path, kPage1Title);
+
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
 
   fuchsia::accessibility::semantics::Node* node =
@@ -343,3 +320,26 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, Slider) {
   EXPECT_TRUE(node->has_states() && node->states().has_range_value());
   EXPECT_EQ(node->states().range_value(), kInitialRangeValue + kStepSize);
 }
+
+// This test makes sure that when semantic updates toggle on / off / on, the
+// full semantic tree is sent in the first update when back on.
+IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, TogglesSemanticsUpdates) {
+  LoadPage(kPage1Path, kPage1Title);
+
+  semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
+
+  semantics_manager_.SetSemanticsModeEnabled(false);
+  base::RunLoop().RunUntilIdle();
+
+  ASSERT_FALSE(frame_impl_->web_contents_for_test()
+                   ->IsWebContentsOnlyAccessibilityModeForTesting());
+
+  // The tree gets cleared when semantic updates are off.
+  EXPECT_EQ(semantics_manager_.semantic_tree()->tree_size(), 0u);
+  semantics_manager_.SetSemanticsModeEnabled(true);
+  base::RunLoop().RunUntilIdle();
+  semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage1NodeCount);
+
+  ASSERT_TRUE(frame_impl_->web_contents_for_test()
+                  ->IsWebContentsOnlyAccessibilityModeForTesting());
+}

fuchsia/engine/browser/fake_semantic_tree.cc

@@ -139,3 +139,7 @@ void FakeSemanticTree::CommitUpdates(CommitUpdatesCallback callback) {
 void FakeSemanticTree::NotImplemented_(const std::string& name) {
   NOTIMPLEMENTED() << name;
 }
+
+void FakeSemanticTree::Clear() {
+  nodes_.clear();
+}

fuchsia/engine/browser/fake_semantic_tree.h

@@ -47,6 +47,9 @@ class FakeSemanticTree
   fuchsia::accessibility::semantics::Node* GetNodeFromRole(
       fuchsia::accessibility::semantics::Role role);
 
+  size_t tree_size() const { return nodes_.size(); }
+  void Clear();
+
   // fuchsia::accessibility::semantics::SemanticTree implementation.
   void UpdateSemanticNodes(
       std::vector<fuchsia::accessibility::semantics::Node> nodes) final;

fuchsia/engine/browser/fake_semantics_manager.cc

@@ -13,6 +13,9 @@ FakeSemanticsManager::FakeSemanticsManager() = default;
 FakeSemanticsManager::~FakeSemanticsManager() = default;
 
 void FakeSemanticsManager::SetSemanticsModeEnabled(bool is_enabled) {
+  if (!is_enabled)
+    semantic_tree_.Clear();
+
   listener_->OnSemanticsModeChanged(is_enabled, []() {});
 }