[flutter_releases] Flutter beta 2.10.0-0.2.pre Engine Cherrypicks

lib/web_ui/lib/src/engine/keyboard_binding.dart

@@ -371,9 +371,7 @@ class KeyboardConverter {
       // followed by an immediate cancel event.
       (_shouldSynthesizeCapsLockUp() && event.code! == _kPhysicalCapsLock);
 
-    final int? lastLogicalRecord = _pressingRecords[physicalKey];
-
-    ui.KeyEventType type;
+    final ui.KeyEventType type;
 
     if (_shouldSynthesizeCapsLockUp() && event.code! == _kPhysicalCapsLock) {
       // Case 1: Handle CapsLock on macOS
@@ -399,28 +397,45 @@ class KeyboardConverter {
 
     } else if (isPhysicalDown) {
       // Case 2: Handle key down of normal keys
-      type = ui.KeyEventType.down;
-      if (lastLogicalRecord != null) {
+      if (_pressingRecords[physicalKey] != null) {
         // This physical key is being pressed according to the record.
         if (event.repeat ?? false) {
           // A normal repeated key.
           type = ui.KeyEventType.repeat;
         } else {
           // A non-repeated key has been pressed that has the exact physical key as
-          // a currently pressed one, usually indicating multiple keyboards are
-          // pressing keys with the same physical key, or the up event was lost
-          // during a loss of focus. The down event is ignored.
-          event.preventDefault();
-          return;
+          // a currently pressed one. This can mean one of the following cases:
+          //
+          //  * Multiple keyboards are pressing keys with the same physical key.
+          //  * The up event was lost during a loss of focus.
+          //  * The previous down event was a system shortcut and its release
+          //    was skipped (see `_startGuardingKey`,) such as holding Ctrl and
+          //    pressing V then V, within the "guard window".
+          //
+          // The three cases can't be distinguished, and in the 3rd case, the
+          // latter event must be dispatched as down events for the framework to
+          // correctly recognize and choose to not to handle. Therefore, an up
+          // event is synthesized before it.
+          _dispatchKeyData!(ui.KeyData(
+            timeStamp: timeStamp,
+            type: ui.KeyEventType.up,
+            physical: physicalKey,
+            logical: logicalKey,
+            character: null,
+            synthesized: true,
+          ));
+          _pressingRecords.remove(physicalKey);
+          type = ui.KeyEventType.down;
         }
       } else {
         // This physical key is not being pressed according to the record. It's a
         // normal down event, whether the system event is a repeat or not.
+        type = ui.KeyEventType.down;
       }
 
     } else { // isPhysicalDown is false and not CapsLock
       // Case 2: Handle key up of normal keys
-      if (lastLogicalRecord == null) {
+      if (_pressingRecords[physicalKey] == null) {
         // The physical key has been released before. It indicates multiple
         // keyboards pressed keys with the same physical key. Ignore the up event.
         event.preventDefault();
@@ -430,6 +445,10 @@ class KeyboardConverter {
       type = ui.KeyEventType.up;
     }
 
+    // The _pressingRecords[physicalKey] might have been changed during the last
+    // `if` clause.
+    final int? lastLogicalRecord = _pressingRecords[physicalKey];
+
     final int? nextLogicalRecord;
     switch (type) {
       case ui.KeyEventType.down:

lib/web_ui/test/keyboard_converter_test.dart

@@ -378,7 +378,7 @@ void testMain() {
     converter.handleEvent(keyUpEvent('ShiftLeft', 'Shift', 0, kLocationLeft));
   });
 
-  test('Duplicate down is ignored', () {
+  test('Duplicate down is preceded with synthesized up', () {
     final List<ui.KeyData> keyDataList = <ui.KeyData>[];
     final KeyboardConverter converter = KeyboardConverter((ui.KeyData key) {
       keyDataList.add(key);
@@ -392,15 +392,26 @@ void testMain() {
     );
     expect(preventedDefault, isTrue);
     preventedDefault = false;
-    // A KeyUp of ShiftLeft is missed due to loss of focus.
+    // A KeyUp of ShiftLeft is missed.
 
     keyDataList.clear();
     converter.handleEvent(keyDownEvent('ShiftLeft', 'Shift', kShift, kLocationLeft)
       ..onPreventDefault = onPreventDefault
     );
-    expect(keyDataList, hasLength(1));
-    expect(keyDataList[0].physical, 0);
-    expect(keyDataList[0].logical, 0);
+    expect(keyDataList, hasLength(2));
+    expectKeyData(keyDataList.first,
+      type: ui.KeyEventType.up,
+      physical: kPhysicalShiftLeft,
+      logical: kLogicalShiftLeft,
+      character: null,
+      synthesized: true,
+    );
+    expectKeyData(keyDataList.last,
+      type: ui.KeyEventType.down,
+      physical: kPhysicalShiftLeft,
+      logical: kLogicalShiftLeft,
+      character: null,
+    );
     expect(preventedDefault, isTrue);
 
     keyDataList.clear();

shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java

@@ -37,6 +37,7 @@ public class FlutterSurfaceView extends SurfaceView implements RenderSurface {
 
   private final boolean renderTransparently;
   private boolean isSurfaceAvailableForRendering = false;
+  private boolean isPaused = false;
   private boolean isAttachedToFlutterRenderer = false;
   @Nullable private FlutterRenderer flutterRenderer;
 
@@ -200,6 +201,7 @@ public void attachToRenderer(@NonNull FlutterRenderer flutterRenderer) {
           "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
       connectSurfaceToRenderer();
     }
+    isPaused = false;
   }
 
   /**
@@ -241,6 +243,7 @@ public void pause() {
       // Don't remove the `flutterUiDisplayListener` as `onFlutterUiDisplayed()` will make
       // the `FlutterSurfaceView` visible.
       flutterRenderer = null;
+      isPaused = true;
       isAttachedToFlutterRenderer = false;
     } else {
       Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
@@ -253,8 +256,13 @@ private void connectSurfaceToRenderer() {
       throw new IllegalStateException(
           "connectSurfaceToRenderer() should only be called when flutterRenderer and getHolder() are non-null.");
     }
-
-    flutterRenderer.startRenderingToSurface(getHolder().getSurface());
+    // When connecting the surface to the renderer, it's possible that the surface is currently
+    // paused. For instance, when a platform view is displayed, the current FlutterSurfaceView
+    // is paused, and rendering continues in a FlutterImageView buffer while the platform view
+    // is displayed.
+    //
+    // startRenderingToSurface stops rendering to an active surface if it isn't paused.
+    flutterRenderer.startRenderingToSurface(getHolder().getSurface(), isPaused);
   }
 
   // FlutterRenderer must be non-null.

shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java

@@ -36,6 +36,7 @@ public class FlutterTextureView extends TextureView implements RenderSurface {
 
   private boolean isSurfaceAvailableForRendering = false;
   private boolean isAttachedToFlutterRenderer = false;
+  private boolean isPaused = false;
   @Nullable private FlutterRenderer flutterRenderer;
   @Nullable private Surface renderSurface;
 
@@ -187,6 +188,7 @@ public void detachFromRenderer() {
   public void pause() {
     if (flutterRenderer != null) {
       flutterRenderer = null;
+      isPaused = true;
       isAttachedToFlutterRenderer = false;
     } else {
       Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
@@ -217,7 +219,8 @@ private void connectSurfaceToRenderer() {
     }
 
     renderSurface = new Surface(getSurfaceTexture());
-    flutterRenderer.startRenderingToSurface(renderSurface);
+    flutterRenderer.startRenderingToSurface(renderSurface, isPaused);
+    isPaused = false;
   }
 
   // FlutterRenderer must be non-null.

shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java

@@ -134,7 +134,7 @@ public void onEngineWillDestroy() {
    *
    * <p>A new {@code FlutterEngine} will not display any UI until a {@link RenderSurface} is
    * registered. See {@link #getRenderer()} and {@link
-   * FlutterRenderer#startRenderingToSurface(Surface)}.
+   * FlutterRenderer#startRenderingToSurface(Surface, boolean)}.
    *
    * <p>A new {@code FlutterEngine} automatically attaches all plugins. See {@link #getPlugins()}.
    *

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

@@ -221,11 +221,24 @@ public void run() {
    * Notifies Flutter that the given {@code surface} was created and is available for Flutter
    * rendering.
    *
+   * <p>If called more than once, the current native resources are released. This can be undesired
+   * if the Engine expects to reuse this surface later. For example, this is true when platform
+   * views are displayed in a frame, and then removed in the next frame.
+   *
+   * <p>To avoid releasing the current surface resources, set {@code keepCurrentSurface} to true.
+   *
    * <p>See {@link android.view.SurfaceHolder.Callback} and {@link
    * android.view.TextureView.SurfaceTextureListener}
+   *
+   * @param surface The render surface.
+   * @param keepCurrentSurface True if the current active surface should not be released.
    */
-  public void startRenderingToSurface(@NonNull Surface surface) {
-    if (this.surface != null) {
+  public void startRenderingToSurface(@NonNull Surface surface, boolean keepCurrentSurface) {
+    // Don't stop rendering the surface if it's currently paused.
+    // Stop rendering to the surface releases the associated native resources, which
+    // causes a glitch when showing platform views.
+    // For more, https://github.com/flutter/flutter/issues/95343
+    if (this.surface != null && !keepCurrentSurface) {
       stopRenderingToSurface();
     }
 
@@ -248,8 +261,8 @@ public void swapSurface(@NonNull Surface surface) {
 
   /**
    * Notifies Flutter that a {@code surface} previously registered with {@link
-   * #startRenderingToSurface(Surface)} has changed size to the given {@code width} and {@code
-   * height}.
+   * #startRenderingToSurface(Surface, boolean)} has changed size to the given {@code width} and
+   * {@code height}.
    *
    * <p>See {@link android.view.SurfaceHolder.Callback} and {@link
    * android.view.TextureView.SurfaceTextureListener}
@@ -260,8 +273,8 @@ public void surfaceChanged(int width, int height) {
 
   /**
    * Notifies Flutter that a {@code surface} previously registered with {@link
-   * #startRenderingToSurface(Surface)} has been destroyed and needs to be released and cleaned up
-   * on the Flutter side.
+   * #startRenderingToSurface(Surface, boolean)} has been destroyed and needs to be released and
+   * cleaned up on the Flutter side.
    *
    * <p>See {@link android.view.SurfaceHolder.Callback} and {@link
    * android.view.TextureView.SurfaceTextureListener}

shell/platform/android/io/flutter/embedding/engine/renderer/RenderSurface.java

@@ -37,7 +37,7 @@ public interface RenderSurface {
    * FlutterRenderer} at the appropriate times:
    *
    * <ol>
-   *   <li>{@link FlutterRenderer#startRenderingToSurface(Surface)}
+   *   <li>{@link FlutterRenderer#startRenderingToSurface(Surface, boolean)}
    *   <li>{@link FlutterRenderer#surfaceChanged(int, int)}}
    *   <li>{@link FlutterRenderer#stopRenderingToSurface()}
    * </ol>

shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java

@@ -6,6 +6,7 @@
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -45,7 +46,7 @@ public void itForwardsSurfaceCreationNotificationToFlutterJNI() {
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
     // Execute the behavior under test.
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Verify the behavior under test.
     verify(fakeFlutterJNI, times(1)).onSurfaceCreated(eq(fakeSurface));
@@ -57,7 +58,7 @@ public void itForwardsSurfaceChangeNotificationToFlutterJNI() {
     Surface fakeSurface = mock(Surface.class);
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.surfaceChanged(100, 50);
@@ -72,7 +73,7 @@ public void itForwardsSurfaceDestructionNotificationToFlutterJNI() {
     Surface fakeSurface = mock(Surface.class);
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.stopRenderingToSurface();
@@ -87,10 +88,10 @@ public void itStopsRenderingToOneSurfaceBeforeRenderingToANewSurface() {
     Surface fakeSurface2 = mock(Surface.class);
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute behavior under test.
-    flutterRenderer.startRenderingToSurface(fakeSurface2);
+    flutterRenderer.startRenderingToSurface(fakeSurface2, /*keepCurrentSurface=*/ false);
 
     // Verify behavior under test.
     verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed(); // notification of 1st surface's removal.
@@ -101,7 +102,7 @@ public void itStopsRenderingToSurfaceWhenRequested() {
     // Setup the test.
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.stopRenderingToSurface();
@@ -110,6 +111,32 @@ public void itStopsRenderingToSurfaceWhenRequested() {
     verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed();
   }
 
+  @Test
+  public void iStopsRenderingToSurfaceWhenSurfaceAlreadySet() {
+    // Setup the test.
+    FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
+
+    // Verify behavior under test.
+    verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed();
+  }
+
+  @Test
+  public void itNeverStopsRenderingToSurfaceWhenRequested() {
+    // Setup the test.
+    FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ true);
+
+    // Verify behavior under test.
+    verify(fakeFlutterJNI, never()).onSurfaceDestroyed();
+  }
+
   @Test
   public void itStopsSurfaceTextureCallbackWhenDetached() {
     // Setup the test.
@@ -120,7 +147,7 @@ public void itStopsSurfaceTextureCallbackWhenDetached() {
     FlutterRenderer.SurfaceTextureRegistryEntry entry =
         (FlutterRenderer.SurfaceTextureRegistryEntry) flutterRenderer.createSurfaceTexture();
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.stopRenderingToSurface();
@@ -143,7 +170,7 @@ public void itRegistersExistingSurfaceTexture() {
         (FlutterRenderer.SurfaceTextureRegistryEntry)
             flutterRenderer.registerSurfaceTexture(surfaceTexture);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Verify behavior under test.
     assertEquals(surfaceTexture, entry.surfaceTexture());
@@ -164,7 +191,7 @@ public void itUnregistersTextureWhenSurfaceTextureFinalized() {
         (FlutterRenderer.SurfaceTextureRegistryEntry) flutterRenderer.createSurfaceTexture();
     long id = entry.id();
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     runFinalization(entry);
@@ -190,7 +217,7 @@ public void itStopsUnregisteringTextureWhenDetached() {
         (FlutterRenderer.SurfaceTextureRegistryEntry) flutterRenderer.createSurfaceTexture();
     long id = entry.id();
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     flutterRenderer.stopRenderingToSurface();
 

shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java

@@ -1,9 +1,11 @@
 package io.flutter.plugin.platform;
 
+import static android.os.Looper.getMainLooper;
 import static io.flutter.embedding.engine.systemchannels.PlatformViewsChannel.PlatformViewTouch;
 import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
+import static org.robolectric.Shadows.shadowOf;
 
 import android.content.Context;
 import android.content.res.AssetManager;
@@ -486,10 +488,7 @@ public void onEndFrame__destroysOverlaySurfaceAfterFrameOnFlutterSurfaceView() {
     platformViewsController.onBeginFrame();
     platformViewsController.onEndFrame();
 
-    verify(overlayImageView, never()).detachFromRenderer();
-
-    // Simulate first frame from the framework.
-    jni.onFirstFrame();
+    shadowOf(getMainLooper()).idle();
     verify(overlayImageView, times(1)).detachFromRenderer();
   }