[web] Fix resizeToAvoidBottomInset on Android web (#179581)

Fixes https://github.com/flutter/flutter/issues/175074

There was an implicit expectation in the resizing code: When the virtual
keyboard is up on mobile, the view's `physicalSize` remains fixed, and
any resizes (presumably caused by the virtual keyboard itself) should be
considered as `viewInsets`.

This expectation was broken inadvertently by my PR:
https://github.com/flutter/flutter/pull/172493

<hr>

The sequence of events that lead to the reported issue:

1. View's physical size is calculated based on window size.
2. Text editing starts, virtual keyboard comes up.
3. View's physical size remains unchanged, and the difference caused by
the keyboard is reported as view insets to the framework.
4. (so far so good).
5. When `resizeToAvoidBottomInset` is true, the framework will re-render
its content to fit the available size.
6. The new render call comes with a new size that's basically `physical
height - bottom inset`.
7. The engine takes that new size and applies it to the DOM.
8.  (now the view's DOM element has been resized incorrectly).
9. ...
10. Eventually, this leads to the new insets being calculated
incorrectly resulting in negative insets which cause the framework to
throw.

<hr>

The fix involves the following:
1. Respect the expectation mentioned above: when the keyboard is up,
view's physical size should remain unchanged.
2. *_If_* we ever end up with negative insets, catch it earlier in the
engine so it's easier to debug the root cause.
3. New regression test.

engine/src/flutter/lib/web_ui/lib/src/engine/window.dart

@@ -69,7 +69,7 @@ class EngineFlutterView implements ui.FlutterView {
     // hot restart.
     embeddingStrategy.attachViewRoot(dom.rootElement);
     pointerBinding = PointerBinding(this);
-    _resizeSubscription = onResize.listen(_didResize);
+    _resizeSubscription = onResize.listen(_handleBrowserResize);
     _globalHtmlAttributes.applyAttributes(
       viewId: viewId,
       rendererTag: renderer.rendererTag,
@@ -120,7 +120,7 @@ class EngineFlutterView implements ui.FlutterView {
   void render(ui.Scene scene, {ui.Size? size}) {
     assert(!isDisposed, 'Trying to render a disposed EngineFlutterView.');
     if (size != null) {
-      resize(size);
+      handleFrameworkResize(size);
     }
     platformDispatcher.render(scene, this);
   }
@@ -208,15 +208,27 @@ class EngineFlutterView implements ui.FlutterView {
   /// hiding `overflow`). Flutter does not attempt to interpret the styles of
   /// `hostElement` to compute its `physicalConstraints`, only its current size.
   @visibleForTesting
-  void resize(ui.Size newPhysicalSize) {
+  void handleFrameworkResize(ui.Size newPhysicalSize) {
+    // TODO(mdebbar): This resizing is only needed for the multiview mode. Should we make it a no-op
+    //                in the full page mode to avoid doing unnecessary work?
+
     // The browser uses CSS, and CSS operates in logical sizes.
     final ui.Size logicalSize = newPhysicalSize / devicePixelRatio;
     dom.rootElement.style
       ..width = '${logicalSize.width}px'
       ..height = '${logicalSize.height}px';
 
-    // Force an update of the physicalSize so it's ready for the renderer.
-    _physicalSize = _computePhysicalSize();
+    // When the keyboard is active on mobile, we explicitly do not update
+    // `_physicalSize`. This is because `_handleBrowserResize` (the method
+    // that handles browser-initiated resizes) has special logic to
+    // keep `_physicalSize` stale (large) and instead rely on `viewInsets`
+    // to shrink the visible area. If `_handleFrameworkResize` were to update `_physicalSize`
+    // to the smaller visible size, it would break the `viewInsets` logic
+    // on subsequent calculations.
+    if (!_shouldPreservePhysicalSizeOnResize) {
+      // Force an update of the physicalSize so it's ready for the renderer.
+      _physicalSize = _computePhysicalSize();
+    }
   }
 
   /// Lazily populated and cleared at the end of the frame.
@@ -274,7 +286,14 @@ class EngineFlutterView implements ui.FlutterView {
 
   Stream<ui.Size?> get onResize => dimensionsProvider.onResize;
 
-  /// Called immediately after the view has been resized.
+  /// Whether to preserve the physical size of the view when the browser window
+  /// resizes.
+  ///
+  /// This is used to prevent the view from resizing when the on-screen keyboard
+  /// appears on mobile devices.
+  bool get _shouldPreservePhysicalSizeOnResize => isMobile && textEditing.isEditing;
+
+  /// Called immediately after the view has been resized by the browser.
   ///
   /// When there is a text editing going on in mobile devices, do not change
   /// the physicalSize, change the [window.viewInsets]. See:
@@ -283,12 +302,17 @@ class EngineFlutterView implements ui.FlutterView {
   ///
   /// Note: always check for rotations for a mobile device. Update the physical
   /// size if the change is caused by a rotation.
-  void _didResize(ui.Size? newSize) {
+  ///
+  /// When `_shouldPreservePhysicalSizeOnResize` is true (i.e., keyboard is active
+  /// on mobile), `_physicalSize` is deliberately kept stale (representing the
+  /// full screen size) while `viewInsets` are updated to reflect the keyboard's
+  /// presence. This allows the framework to correctly shrink its content using
+  /// `resizeToAvoidBottomInset`. When the keyboard is dismissed, `_physicalSize`
+  /// is updated to the actual new physical size of the window.
+  void _handleBrowserResize(ui.Size? _) {
     StyleManager.scaleSemanticsHost(dom.semanticsHost, devicePixelRatio);
     final ui.Size newPhysicalSize = _computePhysicalSize();
-    final bool isEditingOnMobile =
-        isMobile && !_isRotation(newPhysicalSize) && textEditing.isEditing;
-    if (isEditingOnMobile) {
+    if (_shouldPreservePhysicalSizeOnResize && !_isRotation(newPhysicalSize)) {
       _computeOnScreenKeyboardInsets(true);
     } else {
       _physicalSize = newPhysicalSize;
@@ -338,6 +362,13 @@ class EngineFlutterView implements ui.FlutterView {
       _physicalSize!.height,
       isEditingOnMobile,
     );
+
+    // Ensure that viewInsets are never negative. If it's not caught here, it will be caught later
+    // in the framework, and will be hard to debug since the root cause is here.
+    assert(
+      _viewInsets.isNonNegative,
+      'ViewInsets cannot be negative. This is usually caused by an incorrect physicalSize calculation when the keyboard is being dismissed.',
+    );
   }
 }
 
@@ -707,6 +738,9 @@ class ViewPadding implements ui.ViewPadding {
   final double right;
   @override
   final double bottom;
+
+  /// Returns true if all padding values are non-negative.
+  bool get isNonNegative => left >= 0.0 && top >= 0.0 && right >= 0.0 && bottom >= 0.0;
 }
 
 class ViewConstraints implements ui.ViewConstraints {

engine/src/flutter/lib/web_ui/test/engine/window_test.dart

@@ -11,6 +11,7 @@ import 'package:test/bootstrap/browser.dart';
 import 'package:test/test.dart';
 import 'package:ui/src/engine.dart';
 import 'package:ui/ui.dart' as ui;
+import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
 
 import '../common/matchers.dart';
 import '../common/test_initialization.dart';
@@ -25,7 +26,7 @@ void main() {
   internalBootstrapBrowserTest(() => testMain);
 }
 
-Future<void> testMain() async {
+void testMain() {
   setUpImplicitView();
 
   test('onTextScaleFactorChanged preserves the zone', () {
@@ -520,33 +521,30 @@ Future<void> testMain() async {
     domWindow['screen'] = original;
   });
 
-  test(
-    'SingletonFlutterWindow implements locale, locales, and locale change notifications',
-    () async {
-      // This will count how many times we notified about locale changes.
-      var localeChangedCount = 0;
-      myWindow.onLocaleChanged = () {
-        localeChangedCount += 1;
-      };
+  test('SingletonFlutterWindow implements locale, locales, and locale change notifications', () {
+    // This will count how many times we notified about locale changes.
+    var localeChangedCount = 0;
+    myWindow.onLocaleChanged = () {
+      localeChangedCount += 1;
+    };
 
-      // We populate the initial list of locales automatically (only test that we
-      // got some locales; some contributors may be in different locales, so we
-      // can't test the exact contents).
-      expect(myWindow.locale, isA<ui.Locale>());
-      expect(myWindow.locales, isNotEmpty);
+    // We populate the initial list of locales automatically (only test that we
+    // got some locales; some contributors may be in different locales, so we
+    // can't test the exact contents).
+    expect(myWindow.locale, isA<ui.Locale>());
+    expect(myWindow.locales, isNotEmpty);
 
-      // Trigger a change notification (reset locales because the notification
-      // doesn't actually change the list of languages; the test only observes
-      // that the list is populated again).
-      EnginePlatformDispatcher.instance.debugResetLocales();
-      expect(myWindow.locales, isEmpty);
-      expect(myWindow.locale, equals(const ui.Locale.fromSubtags()));
-      expect(localeChangedCount, 0);
-      domWindow.dispatchEvent(createDomEvent('Event', 'languagechange'));
-      expect(myWindow.locales, isNotEmpty);
-      expect(localeChangedCount, 1);
-    },
-  );
+    // Trigger a change notification (reset locales because the notification
+    // doesn't actually change the list of languages; the test only observes
+    // that the list is populated again).
+    EnginePlatformDispatcher.instance.debugResetLocales();
+    expect(myWindow.locales, isEmpty);
+    expect(myWindow.locale, equals(const ui.Locale.fromSubtags()));
+    expect(localeChangedCount, 0);
+    domWindow.dispatchEvent(createDomEvent('Event', 'languagechange'));
+    expect(myWindow.locales, isNotEmpty);
+    expect(localeChangedCount, 1);
+  });
 
   test('dispatches browser event on flutter/service_worker channel', () async {
     final completer = Completer<void>();
@@ -741,7 +739,7 @@ Future<void> testMain() async {
         ..height = 'auto';
 
       // Resize the host to 20x20 (physical pixels).
-      view.resize(const ui.Size.square(50));
+      view.handleFrameworkResize(const ui.Size.square(50));
 
       // The view's physicalSize should be updated too.
       expect(view.physicalSize, const ui.Size(50.0, 50.0));
@@ -775,7 +773,7 @@ Future<void> testMain() async {
       EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(null);
     });
 
-    test('JsViewConstraints are passed and used to compute physicalConstraints', () async {
+    test('JsViewConstraints are passed and used to compute physicalConstraints', () {
       view = EngineFlutterView(
         EnginePlatformDispatcher.instance,
         host,
@@ -799,4 +797,28 @@ Future<void> testMain() async {
       );
     });
   });
+
+  group('keyboard resize behavior', () {
+    setUp(() {
+      // Simulate keyboard being up.
+      textEditing.isEditing = true;
+      ui_web.browser.debugOperatingSystemOverride = ui_web.OperatingSystem.android;
+    });
+
+    tearDown(() {
+      textEditing.isEditing = false;
+      ui_web.browser.debugOperatingSystemOverride = null;
+    });
+
+    test('physicalSize remains unchanged when keyboard is up', () {
+      final ui.Size initialPhysicalSize = myWindow.physicalSize;
+
+      // Pick a smaller size.
+      final ui.Size newSize = initialPhysicalSize ~/ 2;
+      myWindow.handleFrameworkResize(newSize);
+
+      // View's `physicalSize` should remain unchanged.
+      expect(myWindow.physicalSize, initialPhysicalSize);
+    });
+  });
 }