Fix a legacy TODO (flutter#77454) (flutter#79061)

Author: Hixie

dev/integration_tests/web_e2e_tests/test_driver/text_editing_integration.dart

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 // @dart = 2.9
+
 import 'dart:html';
 import 'dart:js_util' as js_util;
+
 import 'package:flutter/gestures.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -21,9 +23,6 @@ void main() {
     app.main();
     await tester.pumpAndSettle();
 
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/51885
-    SystemChannels.textInput.setMockMethodCallHandler(null);
-
     // Focus on a TextFormField.
     final Finder finder = find.byKey(const Key('input'));
     expect(finder, findsOneWidget);
@@ -49,9 +48,6 @@ void main() {
     app.main();
     await tester.pumpAndSettle();
 
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/51885
-    SystemChannels.textInput.setMockMethodCallHandler(null);
-
     // Focus on a TextFormField.
     final Finder finder = find.byKey(const Key('empty-input'));
     expect(finder, findsOneWidget);
@@ -77,9 +73,6 @@ void main() {
     app.main();
     await tester.pumpAndSettle();
 
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/51885
-    SystemChannels.textInput.setMockMethodCallHandler(null);
-
     // This text will show no-enter initially. It will have 'enter-pressed'
     // after `onFieldSubmitted` of TextField is triggered.
     final Finder textFinder = find.byKey(const Key('text'));
@@ -113,9 +106,6 @@ void main() {
     app.main();
     await tester.pumpAndSettle();
 
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/51885
-    SystemChannels.textInput.setMockMethodCallHandler(null);
-
     // Focus on a TextFormField.
     final Finder finder = find.byKey(const Key('input'));
     expect(finder, findsOneWidget);
@@ -148,9 +138,6 @@ void main() {
     app.main();
     await tester.pumpAndSettle();
 
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/51885
-    SystemChannels.textInput.setMockMethodCallHandler(null);
-
     // Focus on a TextFormField.
     final Finder finder = find.byKey(const Key('input'));
     expect(finder, findsOneWidget);
@@ -198,9 +185,6 @@ void main() {
     app.main();
     await tester.pumpAndSettle();
 
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/51885
-    SystemChannels.textInput.setMockMethodCallHandler(null);
-
     // Select something from the selectable text.
     final Finder finder = find.byKey(const Key('selectable'));
     expect(finder, findsOneWidget);

packages/flutter/lib/src/services/system_channels.dart

@@ -120,6 +120,11 @@ class SystemChannels {
   /// they apply, so that stale messages referencing past transactions can be
   /// ignored.
   ///
+  /// In debug builds, messages sent with a client ID of -1 are always accepted.
+  /// This allows tests to smuggle messages without having to mock the engine's
+  /// text handling (for example, allowing the engine to still handle the text
+  /// input messages in an integration test).
+  ///
   /// The methods described below are wrapped in a more convenient form by the
   /// [TextInput] and [TextInputConnection] class.
   ///
@@ -152,9 +157,15 @@ class SystemChannels {
   /// is a transaction identifier. Calls for stale transactions should be ignored.
   ///
   ///  * `TextInputClient.updateEditingState`: The user has changed the contents
-  ///    of the text control. The second argument is a [String] containing a
-  ///    JSON-encoded object with seven keys, in the form expected by
-  ///    [TextEditingValue.fromJSON].
+  ///    of the text control. The second argument is an object with seven keys,
+  ///    in the form expected by [TextEditingValue.fromJSON].
+  ///
+  ///  * `TextInputClient.updateEditingStateWithTag`: One or more text controls
+  ///    were autofilled by the platform's autofill service. The first argument
+  ///    (the client ID) is ignored, the second argument is a map of tags to
+  ///    objects in the form expected by [TextEditingValue.fromJSON]. See
+  ///    [AutofillScope.getAutofillClient] for details on the interpretation of
+  ///    the tag.
   ///
   ///  * `TextInputClient.performAction`: The user has triggered an action. The
   ///    second argument is a [String] consisting of the stringification of one
@@ -165,7 +176,8 @@ class SystemChannels {
   ///    one. The framework should call `TextInput.setClient` and
   ///    `TextInput.setEditingState` again with its most recent information. If
   ///    there is no existing state on the framework side, the call should
-  ///    fizzle.
+  ///    fizzle. (This call is made without a client ID; indeed, without any
+  ///    arguments at all.)
   ///
   ///  * `TextInputClient.onConnectionClosed`: The text input connection closed
   ///    on the platform side. For example the application is moved to

packages/flutter/lib/src/services/text_input.dart

@@ -1327,9 +1327,11 @@ class TextInput {
 
     final List<dynamic> args = methodCall.arguments as List<dynamic>;
 
+    // The updateEditingStateWithTag request (autofill) can come up even to a
+    // text field that doesn't have a connection.
     if (method == 'TextInputClient.updateEditingStateWithTag') {
+      assert(_currentConnection!._client != null);
       final TextInputClient client = _currentConnection!._client;
-      assert(client != null);
       final AutofillScope? scope = client.currentAutofillScope;
       final Map<String, dynamic> editingValue = args[1] as Map<String, dynamic>;
       for (final String tag in editingValue.keys) {
@@ -1343,9 +1345,22 @@ class TextInput {
     }
 
     final int client = args[0] as int;
-    // The incoming message was for a different client.
-    if (client != _currentConnection!._id)
-      return;
+    if (client != _currentConnection!._id) {
+      // If the client IDs don't match, the incoming message was for a different
+      // client.
+      bool debugAllowAnyway = false;
+      assert(() {
+        // In debug builds we allow "-1" as a magical client ID that ignores
+        // this verification step so that tests can always get through, even
+        // when they are not mocking the engine side of text input.
+        if (client == -1)
+          debugAllowAnyway = true;
+        return true;
+      }());
+      if (!debugAllowAnyway)
+        return;
+    }
+
     switch (method) {
       case 'TextInputClient.updateEditingState':
         _currentConnection!._client.updateEditingValue(TextEditingValue.fromJSON(args[1] as Map<String, dynamic>));

packages/flutter/lib/src/widgets/editable_text.dart

@@ -2111,7 +2111,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
     if (_hasFocus) {
       _openInputConnection();
     } else {
-      widget.focusNode.requestFocus();
+      widget.focusNode.requestFocus(); // This eventually calls _openInputConnection also, see _handleFocusChanged.
     }
   }
 

packages/flutter_test/lib/src/binding.dart

@@ -195,9 +195,15 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
 
   /// Called by the test framework at the beginning of a widget test to
   /// prepare the binding for the next test.
+  ///
+  /// If [registerTestTextInput] returns true when this method is called,
+  /// the [testTextInput] is configured to simulate the keyboard.
   void reset() {
     _restorationManager = null;
     resetGestureBinding();
+    testTextInput.reset();
+    if (registerTestTextInput)
+      _testTextInput.register();
   }
 
   @override
@@ -237,14 +243,28 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
   @protected
   bool get overrideHttpClient => true;
 
-  /// Determines whether the binding automatically registers [testTextInput].
+  /// Determines whether the binding automatically registers [testTextInput] as
+  /// a fake keyboard implementation.
   ///
   /// Unit tests make use of this to mock out text input communication for
   /// widgets. An integration test would set this to false, to test real IME
   /// or keyboard input.
   ///
   /// [TestTextInput.isRegistered] reports whether the text input mock is
   /// registered or not.
+  ///
+  /// Some of the properties and methods on [testTextInput] are only valid if
+  /// [registerTestTextInput] returns true when a test starts. If those
+  /// members are accessed when using a binding that sets this flag to false,
+  /// they will throw.
+  ///
+  /// If this property returns true when a test ends, the [testTextInput] is
+  /// unregistered.
+  ///
+  /// This property should not change the value it returns during the lifetime
+  /// of the binding. Changing the value of this property risks very confusing
+  /// behavior as the [TestTextInput] may be inconsistently registered or
+  /// unregistered.
   @protected
   bool get registerTestTextInput => true;
 
@@ -319,9 +339,6 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
       binding.setupHttpOverrides();
     }
     _testTextInput = TestTextInput(onCleared: _resetFocusedEditable);
-    if (registerTestTextInput) {
-      _testTextInput.register();
-    }
   }
 
   @override
@@ -515,12 +532,20 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
   TestTextInput get testTextInput => _testTextInput;
   late TestTextInput _testTextInput;
 
-  /// The current client of the onscreen keyboard. Callers must pump
-  /// an additional frame after setting this property to complete the
-  /// focus change.
+  /// The [State] of the current [EditableText] client of the onscreen keyboard.
+  ///
+  /// Setting this property to a new value causes the given [EditableTextState]
+  /// to focus itself and request the keyboard to establish a
+  /// [TextInputConnection].
+  ///
+  /// Callers must pump an additional frame after setting this property to
+  /// complete the focus change.
   ///
   /// Instead of setting this directly, consider using
   /// [WidgetTester.showKeyboard].
+  //
+  // TODO(ianh): We should just remove this property and move the call to
+  // requestKeyboard to the WidgetTester.showKeyboard method.
   EditableTextState? get focusedEditable => _focusedEditable;
   EditableTextState? _focusedEditable;
   set focusedEditable(EditableTextState? value) {
@@ -799,6 +824,8 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
       // alone so that we don't cause more spurious errors.
       runApp(Container(key: UniqueKey(), child: _postTestMessage)); // Unmount any remaining widgets.
       await pump();
+      if (registerTestTextInput)
+        _testTextInput.unregister();
       invariantTester();
       _verifyAutoUpdateGoldensUnset(autoUpdateGoldensBeforeTest && !isBrowser);
       _verifyReportTestExceptionUnset(reportTestExceptionBeforeTest);