Fix a race in the embedder accessibility unit test (flutter#9585)

johnsonmh · Jun 30, 2019 · b3fe3e9 · b3fe3e9
1 parent 55818f6
commit b3fe3e9
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 23 deletions.
diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart
@@ -45,9 +45,9 @@ Float64List kTestTransform = () {
 }();
 
 void signalNativeTest() native 'SignalNativeTest';
-void notifySemanticsEnabled(bool enabled) native 'NotifyTestData';
-void notifyAccessibilityFeatures(bool reduceMotion) native 'NotifyTestData';
-void notifySemanticsAction(int nodeId, int action, List<int> data) native 'NotifyTestData';
+void notifySemanticsEnabled(bool enabled) native 'NotifySemanticsEnabled';
+void notifyAccessibilityFeatures(bool reduceMotion) native 'NotifyAccessibilityFeatures';
+void notifySemanticsAction(int nodeId, int action, List<int> data) native 'NotifySemanticsAction';
 
 /// Returns a future that completes when `window.onSemanticsEnabledChanged`
 /// fires.

diff --git a/shell/platform/embedder/tests/embedder_a11y_unittests.cc b/shell/platform/embedder/tests/embedder_a11y_unittests.cc
@@ -24,7 +24,7 @@ using Embedder11yTest = testing::EmbedderTest;
 // TODO: This test has been disabled as it is flaky (more reproducible in
 // profile more). Multiple calls to a11y changed handler in Dart code is
 // suspected. https://github.com/flutter/flutter/issues/35218
-TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
+TEST_F(Embedder11yTest, A11yTreeIsConsistent) {
   auto& context = GetEmbedderContext();
 
   fml::AutoResetWaitableEvent latch;
@@ -37,13 +37,32 @@ TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
       })));
 
   // Called by test fixture on UI thread to pass data back to this test.
-  NativeEntry callback;
+  NativeEntry notify_semantics_enabled_callback;
   context.AddNativeCallback(
-      "NotifyTestData",
-      CREATE_NATIVE_ENTRY(([&callback](Dart_NativeArguments args) {
-        ASSERT_NE(callback, nullptr);
-        callback(args);
-      })));
+      "NotifySemanticsEnabled",
+      CREATE_NATIVE_ENTRY(
+          ([&notify_semantics_enabled_callback](Dart_NativeArguments args) {
+            ASSERT_NE(notify_semantics_enabled_callback, nullptr);
+            notify_semantics_enabled_callback(args);
+          })));
+
+  NativeEntry notify_accessibility_features_callback;
+  context.AddNativeCallback(
+      "NotifyAccessibilityFeatures",
+      CREATE_NATIVE_ENTRY((
+          [&notify_accessibility_features_callback](Dart_NativeArguments args) {
+            ASSERT_NE(notify_accessibility_features_callback, nullptr);
+            notify_accessibility_features_callback(args);
+          })));
+
+  NativeEntry notify_semantics_action_callback;
+  context.AddNativeCallback(
+      "NotifySemanticsAction",
+      CREATE_NATIVE_ENTRY(
+          ([&notify_semantics_action_callback](Dart_NativeArguments args) {
+            ASSERT_NE(notify_semantics_action_callback, nullptr);
+            notify_semantics_action_callback(args);
+          })));
 
   EmbedderConfigBuilder builder(context);
   builder.SetDartEntrypoint("a11y_main");
@@ -52,7 +71,7 @@ TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
   ASSERT_TRUE(engine.is_valid());
 
   // Wait for initial NotifySemanticsEnabled(false).
-  callback = [&](Dart_NativeArguments args) {
+  notify_semantics_enabled_callback = [&](Dart_NativeArguments args) {
     bool enabled = true;
     auto handle = Dart_GetNativeBooleanArgument(args, 0, &enabled);
     ASSERT_FALSE(Dart_IsError(handle));
@@ -61,8 +80,18 @@ TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
   };
   latch.Wait();
 
+  // Prepare to NotifyAccessibilityFeatures call
+  fml::AutoResetWaitableEvent notify_features_latch;
+  notify_accessibility_features_callback = [&](Dart_NativeArguments args) {
+    bool enabled = true;
+    auto handle = Dart_GetNativeBooleanArgument(args, 0, &enabled);
+    ASSERT_FALSE(Dart_IsError(handle));
+    ASSERT_FALSE(enabled);
+    notify_features_latch.Signal();
+  };
+
   // Enable semantics. Wait for NotifySemanticsEnabled(true).
-  callback = [&](Dart_NativeArguments args) {
+  notify_semantics_enabled_callback = [&](Dart_NativeArguments args) {
     bool enabled = false;
     auto handle = Dart_GetNativeBooleanArgument(args, 0, &enabled);
     ASSERT_FALSE(Dart_IsError(handle));
@@ -74,17 +103,10 @@ TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
   latch.Wait();
 
   // Wait for initial accessibility features (reduce_motion == false)
-  callback = [&](Dart_NativeArguments args) {
-    bool enabled = true;
-    auto handle = Dart_GetNativeBooleanArgument(args, 0, &enabled);
-    ASSERT_FALSE(Dart_IsError(handle));
-    ASSERT_FALSE(enabled);
-    latch.Signal();
-  };
-  latch.Wait();
+  notify_features_latch.Wait();
 
   // Set accessibility features: (reduce_motion == true)
-  callback = [&](Dart_NativeArguments args) {
+  notify_accessibility_features_callback = [&](Dart_NativeArguments args) {
     bool enabled = false;
     auto handle = Dart_GetNativeBooleanArgument(args, 0, &enabled);
     ASSERT_FALSE(Dart_IsError(handle));
@@ -137,7 +159,7 @@ TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
   ASSERT_EQ(1, action_batch_end_count);
 
   // Dispatch a tap to semantics node 42. Wait for NotifySemanticsAction.
-  callback = [&](Dart_NativeArguments args) {
+  notify_semantics_action_callback = [&](Dart_NativeArguments args) {
     int64_t node_id = 0;
     Dart_GetNativeIntegerArgument(args, 0, &node_id);
     ASSERT_EQ(42, node_id);
@@ -164,7 +186,7 @@ TEST_F(Embedder11yTest, DISABLED_A11yTreeIsConsistent) {
   latch.Wait();
 
   // Disable semantics. Wait for NotifySemanticsEnabled(false).
-  callback = [&](Dart_NativeArguments args) {
+  notify_semantics_enabled_callback = [&](Dart_NativeArguments args) {
     bool enabled = true;
     Dart_GetNativeBooleanArgument(args, 0, &enabled);
     ASSERT_FALSE(enabled);