flutter · loic-sharma · Aug 30, 2022
diff --git a/shell/platform/common/accessibility_bridge.cc b/shell/platform/common/accessibility_bridge.cc
@@ -63,7 +63,7 @@ void AccessibilityBridge::CommitUpdates() {
 
   for (size_t i = results.size(); i > 0; i--) {
     for (SemanticsNode node : results[i - 1]) {
-      ConvertFluterUpdate(node, update);
+      ConvertFlutterUpdate(node, update);
     }
   }
 
@@ -191,8 +191,8 @@ void AccessibilityBridge::GetSubTreeList(SemanticsNode target,
   }
 }
 
-void AccessibilityBridge::ConvertFluterUpdate(const SemanticsNode& node,
-                                              ui::AXTreeUpdate& tree_update) {
+void AccessibilityBridge::ConvertFlutterUpdate(const SemanticsNode& node,
+                                               ui::AXTreeUpdate& tree_update) {
   ui::AXNodeData node_data;
   node_data.id = node.id;
   SetRoleFromFlutterUpdate(node_data, node);

diff --git a/shell/platform/common/accessibility_bridge.h b/shell/platform/common/accessibility_bridge.h
@@ -223,8 +223,8 @@ class AccessibilityBridge
 
   void InitAXTree(const ui::AXTreeUpdate& initial_state);
   void GetSubTreeList(SemanticsNode target, std::vector<SemanticsNode>& result);
-  void ConvertFluterUpdate(const SemanticsNode& node,
-                           ui::AXTreeUpdate& tree_update);
+  void ConvertFlutterUpdate(const SemanticsNode& node,
+                            ui::AXTreeUpdate& tree_update);
   void SetRoleFromFlutterUpdate(ui::AXNodeData& node_data,
                                 const SemanticsNode& node);
   void SetStateFromFlutterUpdate(ui::AXNodeData& node_data,

diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc
@@ -1295,88 +1295,110 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
     settings.log_tag = SAFE_ACCESS(args, log_tag, nullptr);
   }
 
-  flutter::PlatformViewEmbedder::UpdateSemanticsNodesCallback
-      update_semantics_nodes_callback = nullptr;
+  FlutterUpdateSemanticsNodeCallback update_semantics_node_callback = nullptr;
   if (SAFE_ACCESS(args, update_semantics_node_callback, nullptr) != nullptr) {
-    update_semantics_nodes_callback =
-        [ptr = args->update_semantics_node_callback,
-         user_data](flutter::SemanticsNodeUpdates update) {
-          for (const auto& value : update) {
-            const auto& node = value.second;
-            SkMatrix transform = node.transform.asM33();
-            FlutterTransformation flutter_transform{
-                transform.get(SkMatrix::kMScaleX),
-                transform.get(SkMatrix::kMSkewX),
-                transform.get(SkMatrix::kMTransX),
-                transform.get(SkMatrix::kMSkewY),
-                transform.get(SkMatrix::kMScaleY),
-                transform.get(SkMatrix::kMTransY),
-                transform.get(SkMatrix::kMPersp0),
-                transform.get(SkMatrix::kMPersp1),
-                transform.get(SkMatrix::kMPersp2)};
-            const FlutterSemanticsNode embedder_node{
-                sizeof(FlutterSemanticsNode),
-                node.id,
-                static_cast<FlutterSemanticsFlag>(node.flags),
-                static_cast<FlutterSemanticsAction>(node.actions),
-                node.textSelectionBase,
-                node.textSelectionExtent,
-                node.scrollChildren,
-                node.scrollIndex,
-                node.scrollPosition,
-                node.scrollExtentMax,
-                node.scrollExtentMin,
-                node.elevation,
-                node.thickness,
-                node.label.c_str(),
-                node.hint.c_str(),
-                node.value.c_str(),
-                node.increasedValue.c_str(),
-                node.decreasedValue.c_str(),
-                static_cast<FlutterTextDirection>(node.textDirection),
-                FlutterRect{node.rect.fLeft, node.rect.fTop, node.rect.fRight,
-                            node.rect.fBottom},
-                flutter_transform,
-                node.childrenInTraversalOrder.size(),
-                node.childrenInTraversalOrder.data(),
-                node.childrenInHitTestOrder.data(),
-                node.customAccessibilityActions.size(),
-                node.customAccessibilityActions.data(),
-                node.platformViewId,
-            };
-            ptr(&embedder_node, user_data);
-          }
-          const FlutterSemanticsNode batch_end_sentinel = {
-              sizeof(FlutterSemanticsNode),
-              kFlutterSemanticsNodeIdBatchEnd,
-          };
-          ptr(&batch_end_sentinel, user_data);
-        };
+    update_semantics_node_callback = args->update_semantics_node_callback;
   }
 
-  flutter::PlatformViewEmbedder::UpdateSemanticsCustomActionsCallback
-      update_semantics_custom_actions_callback = nullptr;
+  FlutterUpdateSemanticsCustomActionCallback
+      update_semantics_custom_action_callback = nullptr;
   if (SAFE_ACCESS(args, update_semantics_custom_action_callback, nullptr) !=
       nullptr) {
-    update_semantics_custom_actions_callback =
-        [ptr = args->update_semantics_custom_action_callback,
-         user_data](flutter::CustomAccessibilityActionUpdates actions) {
-          for (const auto& value : actions) {
-            const auto& action = value.second;
-            const FlutterSemanticsCustomAction embedder_action = {
+    update_semantics_custom_action_callback =
+        args->update_semantics_custom_action_callback;
+  }
+
+  flutter::PlatformViewEmbedder::UpdateSemanticsCallback
+      update_semantics_callback = nullptr;
+  if (update_semantics_node_callback != nullptr ||
+      update_semantics_custom_action_callback != nullptr) {
+    update_semantics_callback =
+        [update_semantics_node_callback,
+         update_semantics_custom_action_callback,
+         user_data](flutter::SemanticsNodeUpdates update,
+                    flutter::CustomAccessibilityActionUpdates actions) {
+          // First, queue all node and custom action updates.
+          if (update_semantics_node_callback != nullptr) {
+            for (const auto& value : update) {
+              const auto& node = value.second;
+              SkMatrix transform = node.transform.asM33();
+              FlutterTransformation flutter_transform{
+                  transform.get(SkMatrix::kMScaleX),
+                  transform.get(SkMatrix::kMSkewX),
+                  transform.get(SkMatrix::kMTransX),
+                  transform.get(SkMatrix::kMSkewY),
+                  transform.get(SkMatrix::kMScaleY),
+                  transform.get(SkMatrix::kMTransY),
+                  transform.get(SkMatrix::kMPersp0),
+                  transform.get(SkMatrix::kMPersp1),
+                  transform.get(SkMatrix::kMPersp2)};
+              const FlutterSemanticsNode embedder_node{
+                  sizeof(FlutterSemanticsNode),
+                  node.id,
+                  static_cast<FlutterSemanticsFlag>(node.flags),
+                  static_cast<FlutterSemanticsAction>(node.actions),
+                  node.textSelectionBase,
+                  node.textSelectionExtent,
+                  node.scrollChildren,
+                  node.scrollIndex,
+                  node.scrollPosition,
+                  node.scrollExtentMax,
+                  node.scrollExtentMin,
+                  node.elevation,
+                  node.thickness,
+                  node.label.c_str(),
+                  node.hint.c_str(),
+                  node.value.c_str(),
+                  node.increasedValue.c_str(),
+                  node.decreasedValue.c_str(),
+                  static_cast<FlutterTextDirection>(node.textDirection),
+                  FlutterRect{node.rect.fLeft, node.rect.fTop, node.rect.fRight,
+                              node.rect.fBottom},
+                  flutter_transform,
+                  node.childrenInTraversalOrder.size(),
+                  node.childrenInTraversalOrder.data(),
+                  node.childrenInHitTestOrder.data(),
+                  node.customAccessibilityActions.size(),
+                  node.customAccessibilityActions.data(),
+                  node.platformViewId,
+              };
+              update_semantics_node_callback(&embedder_node, user_data);
+            }
+          }
+
+          if (update_semantics_custom_action_callback != nullptr) {
+            for (const auto& value : actions) {
+              const auto& action = value.second;
+              const FlutterSemanticsCustomAction embedder_action = {
+                  sizeof(FlutterSemanticsCustomAction),
+                  action.id,
+                  static_cast<FlutterSemanticsAction>(action.overrideId),
+                  action.label.c_str(),
+                  action.hint.c_str(),
+              };
+              update_semantics_custom_action_callback(&embedder_action,
+                                                      user_data);
+            }
+          }
+
+          // Second, mark node and action batches completed now that all
+          // updates are queued.
+          if (update_semantics_node_callback != nullptr) {
+            const FlutterSemanticsNode batch_end_sentinel = {
+                sizeof(FlutterSemanticsNode),
+                kFlutterSemanticsNodeIdBatchEnd,
+            };
+            update_semantics_node_callback(&batch_end_sentinel, user_data);
+          }
+
+          if (update_semantics_custom_action_callback != nullptr) {
+            const FlutterSemanticsCustomAction batch_end_sentinel = {
                 sizeof(FlutterSemanticsCustomAction),
-                action.id,
-                static_cast<FlutterSemanticsAction>(action.overrideId),
-                action.label.c_str(),
-                action.hint.c_str(),
+                kFlutterSemanticsCustomActionIdBatchEnd,
             };
-            ptr(&embedder_action, user_data);
+            update_semantics_custom_action_callback(&batch_end_sentinel,
+                                                    user_data);
           }
-          const FlutterSemanticsCustomAction batch_end_sentinel = {
-              sizeof(FlutterSemanticsCustomAction),
-              kFlutterSemanticsCustomActionIdBatchEnd,
-          };
-          ptr(&batch_end_sentinel, user_data);
         };
   }
 
@@ -1471,8 +1493,7 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
 
   flutter::PlatformViewEmbedder::PlatformDispatchTable platform_dispatch_table =
       {
-          update_semantics_nodes_callback,            //
-          update_semantics_custom_actions_callback,   //
+          update_semantics_callback,                  //
           platform_message_response_callback,         //
           vsync_callback,                             //
           compute_platform_resolved_locale_callback,  //

diff --git a/shell/platform/embedder/platform_view_embedder.cc b/shell/platform/embedder/platform_view_embedder.cc
@@ -67,13 +67,9 @@ PlatformViewEmbedder::~PlatformViewEmbedder() = default;
 void PlatformViewEmbedder::UpdateSemantics(
     flutter::SemanticsNodeUpdates update,
     flutter::CustomAccessibilityActionUpdates actions) {
-  if (platform_dispatch_table_.update_semantics_nodes_callback != nullptr) {
-    platform_dispatch_table_.update_semantics_nodes_callback(std::move(update));
-  }
-  if (platform_dispatch_table_.update_semantics_custom_actions_callback !=
-      nullptr) {
-    platform_dispatch_table_.update_semantics_custom_actions_callback(
-        std::move(actions));
+  if (platform_dispatch_table_.update_semantics_callback != nullptr) {
+    platform_dispatch_table_.update_semantics_callback(std::move(update),
+                                                       std::move(actions));
   }
 }
 

diff --git a/shell/platform/embedder/platform_view_embedder.h b/shell/platform/embedder/platform_view_embedder.h
@@ -31,10 +31,9 @@ namespace flutter {
 
 class PlatformViewEmbedder final : public PlatformView {
  public:
-  using UpdateSemanticsNodesCallback =
-      std::function<void(flutter::SemanticsNodeUpdates update)>;
-  using UpdateSemanticsCustomActionsCallback =
-      std::function<void(flutter::CustomAccessibilityActionUpdates actions)>;
+  using UpdateSemanticsCallback =
+      std::function<void(flutter::SemanticsNodeUpdates update,
+                         flutter::CustomAccessibilityActionUpdates actions)>;
   using PlatformMessageResponseCallback =
       std::function<void(std::unique_ptr<PlatformMessage>)>;
   using ComputePlatformResolvedLocaleCallback =
@@ -43,9 +42,7 @@ class PlatformViewEmbedder final : public PlatformView {
   using OnPreEngineRestartCallback = std::function<void()>;
 
   struct PlatformDispatchTable {
-    UpdateSemanticsNodesCallback update_semantics_nodes_callback;  // optional
-    UpdateSemanticsCustomActionsCallback
-        update_semantics_custom_actions_callback;  // optional
+    UpdateSemanticsCallback update_semantics_callback;  // optional
     PlatformMessageResponseCallback
         platform_message_response_callback;             // optional
     VsyncWaiterEmbedder::VsyncCallback vsync_callback;  // optional

diff --git a/shell/platform/embedder/tests/embedder_a11y_unittests.cc b/shell/platform/embedder/tests/embedder_a11y_unittests.cc
@@ -120,40 +120,47 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistent) {
   latch.Wait();
 
   // Wait for UpdateSemantics callback on platform (current) thread.
-  int node_count = 0;
   int node_batch_end_count = 0;
-  context.SetSemanticsNodeCallback(
-      [&node_count, &node_batch_end_count](const FlutterSemanticsNode* node) {
-        if (node->id == kFlutterSemanticsNodeIdBatchEnd) {
-          ++node_batch_end_count;
-        } else {
-          ++node_count;
-          ASSERT_EQ(1.0, node->transform.scaleX);
-          ASSERT_EQ(2.0, node->transform.skewX);
-          ASSERT_EQ(3.0, node->transform.transX);
-          ASSERT_EQ(4.0, node->transform.skewY);
-          ASSERT_EQ(5.0, node->transform.scaleY);
-          ASSERT_EQ(6.0, node->transform.transY);
-          ASSERT_EQ(7.0, node->transform.pers0);
-          ASSERT_EQ(8.0, node->transform.pers1);
-          ASSERT_EQ(9.0, node->transform.pers2);
-
-          if (node->id == 128) {
-            ASSERT_EQ(0x3f3, node->platform_view_id);
-          } else {
-            ASSERT_EQ(0, node->platform_view_id);
-          }
-        }
-      });
+  int action_batch_end_count = 0;
+
+  int node_count = 0;
+  context.SetSemanticsNodeCallback([&](const FlutterSemanticsNode* node) {
+    if (node->id == kFlutterSemanticsNodeIdBatchEnd) {
+      ++node_batch_end_count;
+    } else {
+      // Batches should be completed after all nodes are received.
+      ASSERT_EQ(0, node_batch_end_count);
+      ASSERT_EQ(0, action_batch_end_count);
+
+      ++node_count;
+      ASSERT_EQ(1.0, node->transform.scaleX);
+      ASSERT_EQ(2.0, node->transform.skewX);
+      ASSERT_EQ(3.0, node->transform.transX);
+      ASSERT_EQ(4.0, node->transform.skewY);
+      ASSERT_EQ(5.0, node->transform.scaleY);
+      ASSERT_EQ(6.0, node->transform.transY);
+      ASSERT_EQ(7.0, node->transform.pers0);
+      ASSERT_EQ(8.0, node->transform.pers1);
+      ASSERT_EQ(9.0, node->transform.pers2);
+
+      if (node->id == 128) {
+        ASSERT_EQ(0x3f3, node->platform_view_id);
+      } else {
+        ASSERT_EQ(0, node->platform_view_id);
+      }
+    }
+  });
 
   int action_count = 0;
-  int action_batch_end_count = 0;
   context.SetSemanticsCustomActionCallback(
-      [&action_count,
-       &action_batch_end_count](const FlutterSemanticsCustomAction* action) {
+      [&](const FlutterSemanticsCustomAction* action) {
         if (action->id == kFlutterSemanticsCustomActionIdBatchEnd) {
           ++action_batch_end_count;
         } else {
+          // Batches should be completed after all actions are received.
+          ASSERT_EQ(0, node_batch_end_count);
+          ASSERT_EQ(0, action_batch_end_count);
+
           ++action_count;
         }
       });

diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc
@@ -119,14 +119,14 @@ void EmbedderTestContext::AddNativeCallback(const char* name,
 }
 
 void EmbedderTestContext::SetSemanticsNodeCallback(
-    const SemanticsNodeCallback& update_semantics_node_callback) {
-  update_semantics_node_callback_ = update_semantics_node_callback;
+    SemanticsNodeCallback update_semantics_node_callback) {
+  update_semantics_node_callback_ = std::move(update_semantics_node_callback);
 }
 
 void EmbedderTestContext::SetSemanticsCustomActionCallback(
-    const SemanticsActionCallback& update_semantics_custom_action_callback) {
+    SemanticsActionCallback update_semantics_custom_action_callback) {
   update_semantics_custom_action_callback_ =
-      update_semantics_custom_action_callback;
+      std::move(update_semantics_custom_action_callback);
 }
 
 void EmbedderTestContext::SetPlatformMessageCallback(

diff --git a/shell/platform/embedder/tests/embedder_test_context.h b/shell/platform/embedder/tests/embedder_test_context.h
@@ -71,11 +71,10 @@ class EmbedderTestContext {
 
   void AddNativeCallback(const char* name, Dart_NativeFunction function);
 
-  void SetSemanticsNodeCallback(
-      const SemanticsNodeCallback& update_semantics_node);
+  void SetSemanticsNodeCallback(SemanticsNodeCallback update_semantics_node);
 
   void SetSemanticsCustomActionCallback(
-      const SemanticsActionCallback& semantics_custom_action);
+      SemanticsActionCallback semantics_custom_action);
 
   void SetPlatformMessageCallback(
       const std::function<void(const FlutterPlatformMessage*)>& callback);