Add null-aware access to semantics instance (flutter#40146)

* Add null-aware access to semantics instance * Change callsites to use nullable instance * Revert * Add test * whitespace * Typo * Fix comment * Whitespace * Comment fix
mnordine · Mar 14, 2023 · 9adff0a · 9adff0a
1 parent 59e0ced
commit 9adff0a
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 6 deletions.
diff --git a/lib/web_ui/lib/src/engine/semantics/text_field.dart b/lib/web_ui/lib/src/engine/semantics/text_field.dart
@@ -33,8 +33,8 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy {
   /// This method must be called prior to accessing [instance].
   static SemanticsTextEditingStrategy ensureInitialized(
       HybridTextEditing owner) {
-    if (_instance != null && instance.owner == owner) {
-      return instance;
+    if (_instance != null && _instance?.owner == owner) {
+      return _instance!;
     }
     return _instance = SemanticsTextEditingStrategy(owner);
   }
@@ -401,7 +401,7 @@ class TextField extends RoleManager {
         allowInterop((DomEvent event) {
       semanticsObject.element.setAttribute('role', 'textbox');
       activeEditableElement.remove();
-      SemanticsTextEditingStrategy.instance.deactivate(this);
+      SemanticsTextEditingStrategy._instance?.deactivate(this);
 
       // Focus on semantics element before removing the editable element, so that
       // the user can continue navigating the page with the assistive technology.
@@ -430,11 +430,11 @@ class TextField extends RoleManager {
             activeEditableElement.focus();
           });
         }
-        SemanticsTextEditingStrategy.instance.activate(this);
+        SemanticsTextEditingStrategy._instance?.activate(this);
       } else if (flutterViewEmbedder.glassPaneShadow.activeElement ==
           activeEditableElement) {
         if (!isIosSafari) {
-          SemanticsTextEditingStrategy.instance.deactivate(this);
+          SemanticsTextEditingStrategy._instance?.deactivate(this);
           // Only apply text, because this node is not focused.
         }
         activeEditableElement.blur();
@@ -460,6 +460,6 @@ class TextField extends RoleManager {
     if (!isIosSafari) {
       editableElement?.remove();
     }
-    SemanticsTextEditingStrategy.instance.deactivate(this);
+    SemanticsTextEditingStrategy._instance?.deactivate(this);
   }
 }
diff --git a/lib/web_ui/test/engine/semantics/text_field_test.dart b/lib/web_ui/test/engine/semantics/text_field_test.dart
@@ -36,6 +36,33 @@ void testMain() {
     EngineSemanticsOwner.debugResetSemantics();
   });
 
+  group('$SemanticsTextEditingStrategy pre-initialization tests', () {
+    setUp(() {
+      semantics()
+        ..debugOverrideTimestampFunction(() => _testTime)
+        ..semanticsEnabled = true;
+    });
+
+    tearDown(() {
+      semantics().semanticsEnabled = false;
+    });
+
+    test('Calling dispose() pre-initialization will not throw an error', () {
+      final SemanticsObject textFieldSemantics = createTextFieldSemantics(
+        value: 'hi',
+        isFocused: true,
+      );
+      final TextField textField =
+          textFieldSemantics.debugRoleManagerFor(Role.textField)! as TextField;
+
+      // ensureInitialized() isn't called prior to calling dispose() here.
+      // Since we are conditionally calling dispose() on our
+      // SemanticsTextEditingStrategy._instance, we shouldn't expect an error.
+      // ref: https://github.com/flutter/engine/pull/40146
+      expect(() => textField.dispose(), returnsNormally);
+    });
+  });
+
   group('$SemanticsTextEditingStrategy', () {
     late HybridTextEditing testTextEditing;
     late SemanticsTextEditingStrategy strategy;
@@ -867,6 +894,7 @@ void testMain() {
   }, skip: !isSafari);
 }
 
+
 SemanticsObject createTextFieldSemantics({
   required String value,
   String label = '',