[flutter_releases] Flutter beta 2.10.0-0.2.pre Engine Cherrypicks (#30918)

* Impl and test (#30488)

* Fix missing shcore.dll error on Windows 7 (#30699)

* Remove glitch when displaying platform views (#30724)

* Win32: Fix Korean text input (#30805)

Fixes an issue with Korean IMEs wherein a text input state update may be
sent to the framework that misleads the framework into assuming that IME
composing has ended.

When inputting Korean text, characters are built up keystroke by
keystroke until the point that either:
* the user presses space/enter to terminate composing and commit the
  character, or;
* the user presses a key such that the character currently being
  composed cannot be modified further, and the IME determines that the
  user has begun composing the next character.

The following is an example sequence of events for the latter case:

1. User presses ㅂ. GCS_COMPSTR event received with ㅂ. Embedder sends
   state update to framework.
2. User presses ㅏ. GCS_COMPSTR event received with 바. Embedder sends
   state update to framework.
3. User presses ㄴ. GCS_COMPSTR event received with 반. Embedder sends
   state update to framework.
4. User presses ㅏ. At this point, the current character being composed
   (반) cannot be modified in a meaningful way, and the IME determines
   that the user is typing 바 followed by 나. GCS_RESULTSTR event
   received with 바, immediately followed by GCS_COMPSTR event with 나.

In step 4, we previously sent two events to the framework, one
immediately after the other:
* GCS_RESULTSTR triggers the text input model to commit the current
  composing region to the string under edit. This causes the composing
  region to collapse to an empty range.
* GCS_COMPSTR triggers the text input model to insert the new composing
  character and set the composing region to that character.

Conceptually, this is an atomic operation. The fourth keystroke causes
the 반 character to be broken into two (바 and ㄴ) and the latter to be
modified to 나. From the user's point of view, as well as from the IME's
point of view, the user has NOT stopped composing, and the composing
region has simply moved on to the next character.

Flutter has no concept of whether the user is composing or not other
that whether a non-empty composing region exists. As such, sending a
state update after the GCS_RESULTSTR event misleads the framework into
believing that composing has ended. This triggers a serious bug:

Text fields with input formatters applied do not perform input
formatting updates while composing is active; instead they wait until
composing has ended to apply any formatting. The previous behaviour
would thus trigger input formatters to be applied each time the user
input caused a new character to be input. This has the add-on negative
effect that once formatting has been applied, it sends an update back to
the embedder so that the native OS text input state can be updated.
However, since the GCS_RESULTSTR event is _immediately_ followed by a
GCS_COMPSTR, the state has changed in the meantime, and the embedder is
left processing an update (the intermediate state sent after the
GCS_RESULTSTR) which is now out of date (i.e. missing the new state from
the GCS_COMPSTR event).

Since GCS_RESULTR events are always immediately followed by a subsequent
GCS_COMPSTR (in the case where composing continues) or a
WM_IME_ENDCOMPOSITION (in the case where composing is finished), and
because the event handlers for both of those send updated state to the
framework, this change eliminates sending the (intermediate) state in
response to GCS_COMPSTR events.

Issue: flutter/flutter#96209 (full fix)
Issue: flutter/flutter#88645 (partial fix)

* Ensure that PlatformViewIOS does not call into Shell semantics APIs during destruction (#30835)

* Ensure that PlatformViewIOS does not call into Shell semantics APIs during destruction (#30835)

Co-authored-by: Tong Mu <[email protected]>
Co-authored-by: Chris Bracken <[email protected]>
Co-authored-by: Emmanuel Garcia <[email protected]>
Co-authored-by: Jason Simmons <[email protected]>

Author: 5 people

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();
   }