Don't re-initialize the default RenderSurface when returning from hybrid composition mode (flutter#47358)

When we enter hybrid composition mode we 'pause' the default RenderSurface (implemented by SurfaceView or TextureView) and swap to an ImageReader based RenderSurface.

When we return from hybrid composition mode we recreate and re-initialize the real RenderSurface as if it was being used for the first time.

This broke Platform Views in an internal app b/306122497 because we would incorrectly tell the texture to attach when it was never detached.

This CL changes the protocol so that when we return from hybrid composition mode we only swap the RenderSurface and do not re-create it. This avoids doing a bunch of unnecessary work and fixes the logic error of re-attaching textures that were never detached.

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue.*

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: johnmccutchan

shell/common/shell.cc

@@ -814,7 +814,6 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
   const bool should_post_raster_task =
       !task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread();
 
-  fml::AutoResetWaitableEvent latch;
   auto raster_task = fml::MakeCopyable(
       [&waiting_for_first_frame = waiting_for_first_frame_,  //
        rasterizer = rasterizer_->GetWeakPtr(),               //
@@ -841,8 +840,8 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
   // weak pointer. However, we are preventing the platform view from being
   // collected by using a latch.
   auto* platform_view = platform_view_.get();
-
   FML_DCHECK(platform_view);
+  fml::AutoResetWaitableEvent latch;
 
   auto io_task = [io_manager = io_manager_->GetWeakPtr(), platform_view,
                   ui_task_runner = task_runners_.GetUITaskRunner(), ui_task,

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

@@ -186,6 +186,10 @@ public void pause() {
     // Not supported.
   }
 
+  public void resume() {
+    // Not supported.
+  }
+
   /**
    * Acquires the next image to be drawn to the {@link android.graphics.Canvas}. Returns true if
    * there's an image available in the queue.

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

@@ -38,9 +38,12 @@ 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;
 
+  private boolean shouldNotify() {
+    return flutterRenderer != null && !isPaused;
+  }
+
   // Connects the {@code Surface} beneath this {@code SurfaceView} with Flutter's native code.
   // Callbacks are received by this Object and then those messages are forwarded to our
   // FlutterRenderer, and then on to the JNI bridge over to native Flutter code.
@@ -51,7 +54,7 @@ public void surfaceCreated(@NonNull SurfaceHolder holder) {
           Log.v(TAG, "SurfaceHolder.Callback.startRenderingToSurface()");
           isSurfaceAvailableForRendering = true;
 
-          if (isAttachedToFlutterRenderer) {
+          if (shouldNotify()) {
             connectSurfaceToRenderer();
           }
         }
@@ -60,7 +63,7 @@ public void surfaceCreated(@NonNull SurfaceHolder holder) {
         public void surfaceChanged(
             @NonNull SurfaceHolder holder, int format, int width, int height) {
           Log.v(TAG, "SurfaceHolder.Callback.surfaceChanged()");
-          if (isAttachedToFlutterRenderer) {
+          if (shouldNotify()) {
             changeSurfaceSize(width, height);
           }
         }
@@ -70,7 +73,7 @@ public void surfaceDestroyed(@NonNull SurfaceHolder holder) {
           Log.v(TAG, "SurfaceHolder.Callback.stopRenderingToSurface()");
           isSurfaceAvailableForRendering = false;
 
-          if (isAttachedToFlutterRenderer) {
+          if (shouldNotify()) {
             disconnectSurfaceFromRenderer();
           }
         }
@@ -183,25 +186,15 @@ public void attachToRenderer(@NonNull FlutterRenderer flutterRenderer) {
     if (this.flutterRenderer != null) {
       Log.v(
           TAG,
-          "Already connected to a FlutterRenderer. Detaching from old one and attaching to new one.");
+          "Already connected to a FlutterRenderer. Detaching from old one and attaching to new"
+              + " one.");
       this.flutterRenderer.stopRenderingToSurface();
       this.flutterRenderer.removeIsDisplayingFlutterUiListener(flutterUiDisplayListener);
     }
 
     this.flutterRenderer = flutterRenderer;
-    isAttachedToFlutterRenderer = true;
 
-    this.flutterRenderer.addIsDisplayingFlutterUiListener(flutterUiDisplayListener);
-
-    // If we're already attached to an Android window then we're now attached to both a renderer
-    // and the Android window. We can begin rendering now.
-    if (isSurfaceAvailableForRendering) {
-      Log.v(
-          TAG,
-          "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
-      connectSurfaceToRenderer();
-    }
-    isPaused = false;
+    resume();
   }
 
   /**
@@ -222,13 +215,13 @@ public void detachFromRenderer() {
         disconnectSurfaceFromRenderer();
       }
 
+      pause();
+
       // Make the SurfaceView invisible to avoid showing a black rectangle.
       setAlpha(0.0f);
-
       flutterRenderer.removeIsDisplayingFlutterUiListener(flutterUiDisplayListener);
-
       flutterRenderer = null;
-      isAttachedToFlutterRenderer = false;
+
     } else {
       Log.w(TAG, "detachFromRenderer() invoked when no FlutterRenderer was attached.");
     }
@@ -239,22 +232,37 @@ public void detachFromRenderer() {
    * UI to this {@code FlutterSurfaceView}.
    */
   public void pause() {
-    if (flutterRenderer != null) {
-      // Don't remove the `flutterUiDisplayListener` as `onFlutterUiDisplayed()` will make
-      // the `FlutterSurfaceView` visible.
-      flutterRenderer = null;
-      isPaused = true;
-      isAttachedToFlutterRenderer = false;
-    } else {
+    if (flutterRenderer == null) {
       Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
+      return;
+    }
+    isPaused = true;
+  }
+
+  public void resume() {
+    if (flutterRenderer == null) {
+      Log.w(TAG, "resume() invoked when no FlutterRenderer was attached.");
+      return;
+    }
+    this.flutterRenderer.addIsDisplayingFlutterUiListener(flutterUiDisplayListener);
+
+    // If we're already attached to an Android window then we're now attached to both a renderer
+    // and the Android window. We can begin rendering now.
+    if (isSurfaceAvailableForRendering) {
+      Log.v(
+          TAG,
+          "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
+      connectSurfaceToRenderer();
     }
+    isPaused = false;
   }
 
   // FlutterRenderer and getSurfaceTexture() must both be non-null.
   private void connectSurfaceToRenderer() {
     if (flutterRenderer == null || getHolder() == null) {
       throw new IllegalStateException(
-          "connectSurfaceToRenderer() should only be called when flutterRenderer and getHolder() are non-null.");
+          "connectSurfaceToRenderer() should only be called when flutterRenderer and getHolder()"
+              + " are non-null.");
     }
     // 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
@@ -285,9 +293,10 @@ private void changeSurfaceSize(int width, int height) {
   private void disconnectSurfaceFromRenderer() {
     if (flutterRenderer == null) {
       throw new IllegalStateException(
-          "disconnectSurfaceFromRenderer() should only be called when flutterRenderer is non-null.");
+          "disconnectSurfaceFromRenderer() should only be called when flutterRenderer is"
+              + " non-null.");
     }
 
     flutterRenderer.stopRenderingToSurface();
   }
-}
+}

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

@@ -35,11 +35,14 @@ public class FlutterTextureView extends TextureView implements RenderSurface {
   private static final String TAG = "FlutterTextureView";
 
   private boolean isSurfaceAvailableForRendering = false;
-  private boolean isAttachedToFlutterRenderer = false;
   private boolean isPaused = false;
   @Nullable private FlutterRenderer flutterRenderer;
   @Nullable private Surface renderSurface;
 
+  private boolean shouldNotify() {
+    return flutterRenderer != null && !isPaused;
+  }
+
   // Connects the {@code SurfaceTexture} beneath this {@code TextureView} with Flutter's native
   // code.
   // Callbacks are received by this Object and then those messages are forwarded to our
@@ -55,7 +58,7 @@ public void onSurfaceTextureAvailable(
           // If we're already attached to a FlutterRenderer then we're now attached to both a
           // renderer
           // and the Android window, so we can begin rendering now.
-          if (isAttachedToFlutterRenderer) {
+          if (shouldNotify()) {
             connectSurfaceToRenderer();
           }
         }
@@ -64,7 +67,7 @@ public void onSurfaceTextureAvailable(
         public void onSurfaceTextureSizeChanged(
             @NonNull SurfaceTexture surface, int width, int height) {
           Log.v(TAG, "SurfaceTextureListener.onSurfaceTextureSizeChanged()");
-          if (isAttachedToFlutterRenderer) {
+          if (shouldNotify()) {
             changeSurfaceSize(width, height);
           }
         }
@@ -82,7 +85,7 @@ public boolean onSurfaceTextureDestroyed(@NonNull SurfaceTexture surface) {
           // If we're attached to a FlutterRenderer then we need to notify it that our
           // SurfaceTexture
           // has been destroyed.
-          if (isAttachedToFlutterRenderer) {
+          if (shouldNotify()) {
             disconnectSurfaceFromRenderer();
           }
 
@@ -139,21 +142,14 @@ public void attachToRenderer(@NonNull FlutterRenderer flutterRenderer) {
     if (this.flutterRenderer != null) {
       Log.v(
           TAG,
-          "Already connected to a FlutterRenderer. Detaching from old one and attaching to new one.");
+          "Already connected to a FlutterRenderer. Detaching from old one and attaching to new"
+              + " one.");
       this.flutterRenderer.stopRenderingToSurface();
     }
 
     this.flutterRenderer = flutterRenderer;
-    isAttachedToFlutterRenderer = true;
 
-    // If we're already attached to an Android window then we're now attached to both a renderer
-    // and the Android window. We can begin rendering now.
-    if (isSurfaceAvailableForRendering) {
-      Log.v(
-          TAG,
-          "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
-      connectSurfaceToRenderer();
-    }
+    resume();
   }
 
   /**
@@ -174,8 +170,9 @@ public void detachFromRenderer() {
         disconnectSurfaceFromRenderer();
       }
 
+      pause();
+
       flutterRenderer = null;
-      isAttachedToFlutterRenderer = false;
     } else {
       Log.w(TAG, "detachFromRenderer() invoked when no FlutterRenderer was attached.");
     }
@@ -186,13 +183,28 @@ public void detachFromRenderer() {
    * UI to this {@code FlutterTextureView}.
    */
   public void pause() {
-    if (flutterRenderer != null) {
-      flutterRenderer = null;
-      isPaused = true;
-      isAttachedToFlutterRenderer = false;
-    } else {
+    if (flutterRenderer == null) {
       Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
+      return;
     }
+    isPaused = true;
+  }
+
+  public void resume() {
+    if (flutterRenderer == null) {
+      Log.w(TAG, "resume() invoked when no FlutterRenderer was attached.");
+      return;
+    }
+
+    // If we're already attached to an Android window then we're now attached to both a renderer
+    // and the Android window. We can begin rendering now.
+    if (isSurfaceAvailableForRendering) {
+      Log.v(
+          TAG,
+          "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
+      connectSurfaceToRenderer();
+    }
+    isPaused = false;
   }
 
   /**
@@ -209,7 +221,8 @@ public void setRenderSurface(Surface renderSurface) {
   private void connectSurfaceToRenderer() {
     if (flutterRenderer == null || getSurfaceTexture() == null) {
       throw new IllegalStateException(
-          "connectSurfaceToRenderer() should only be called when flutterRenderer and getSurfaceTexture() are non-null.");
+          "connectSurfaceToRenderer() should only be called when flutterRenderer and"
+              + " getSurfaceTexture() are non-null.");
     }
 
     // Definitively release the surface to avoid leaked closeables, just in case
@@ -220,7 +233,6 @@ private void connectSurfaceToRenderer() {
 
     renderSurface = new Surface(getSurfaceTexture());
     flutterRenderer.startRenderingToSurface(renderSurface, isPaused);
-    isPaused = false;
   }
 
   // FlutterRenderer must be non-null.
@@ -243,7 +255,8 @@ private void changeSurfaceSize(int width, int height) {
   private void disconnectSurfaceFromRenderer() {
     if (flutterRenderer == null) {
       throw new IllegalStateException(
-          "disconnectSurfaceFromRenderer() should only be called when flutterRenderer is non-null.");
+          "disconnectSurfaceFromRenderer() should only be called when flutterRenderer is"
+              + " non-null.");
     }
 
     flutterRenderer.stopRenderingToSurface();
@@ -252,4 +265,4 @@ private void disconnectSurfaceFromRenderer() {
       renderSurface = null;
     }
   }
-}
+}

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

@@ -1369,9 +1369,9 @@ public void revertImageView(@NonNull Runnable onDone) {
       onDone.run();
       return;
     }
-    // Start rendering on the previous surface.
+    // Resume rendering to the previous surface.
     // This surface is typically `FlutterSurfaceView` or `FlutterTextureView`.
-    renderSurface.attachToRenderer(renderer);
+    renderSurface.resume();
 
     // Install a Flutter UI listener to wait until the first frame is rendered
     // in the new surface to call the `onDone` callback.

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

@@ -467,20 +467,26 @@ protected void finalize() throws Throwable {
    * android.view.TextureView.SurfaceTextureListener}
    *
    * @param surface The render surface.
-   * @param keepCurrentSurface True if the current active surface should not be released.
+   * @param onlySwap True if the current active surface should not be detached.
    */
-  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) {
+  public void startRenderingToSurface(@NonNull Surface surface, boolean onlySwap) {
+    if (!onlySwap) {
+      // Stop rendering to the surface releases the associated native resources, which
+      // causes a glitch when toggling between rendering to an image view (hybrid composition) and
+      // rendering directly to a Surface or Texture view. For more,
+      // https://github.com/flutter/flutter/issues/95343
       stopRenderingToSurface();
     }
 
     this.surface = surface;
 
-    flutterJNI.onSurfaceCreated(surface);
+    if (onlySwap) {
+      // In the swap case we are just swapping the surface that we render to.
+      flutterJNI.onSurfaceWindowChanged(surface);
+    } else {
+      // In the non-swap case we are creating a new surface to render to.
+      flutterJNI.onSurfaceCreated(surface);
+    }
   }
 
   /**

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

@@ -60,4 +60,11 @@ public interface RenderSurface {
    * #attachToRenderer(FlutterRenderer)}.
    */
   void pause();
+
+  /**
+   * Instructs this {@code RenderSurface} to resume forwarding {@code Surface} notifications to the
+   * {@code FlutterRenderer} that was previously connected with {@link
+   * #attachToRenderer(FlutterRenderer)}.
+   */
+  void resume();
 }

shell/platform/android/platform_view_android.cc

@@ -197,6 +197,8 @@ void PlatformViewAndroid::NotifySurfaceWindowChanged(
         });
     latch.Wait();
   }
+
+  PlatformView::ScheduleFrame();
 }
 
 void PlatformViewAndroid::NotifyDestroyed() {