Ensure all images are closed in FlutterImageView (flutter#20842)

Author: Emmanuel Garcia

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

@@ -4,7 +4,6 @@
 
 package io.flutter.embedding.android;
 
-import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.graphics.Bitmap;
@@ -15,13 +14,16 @@
 import android.media.Image;
 import android.media.Image.Plane;
 import android.media.ImageReader;
+import android.util.AttributeSet;
 import android.view.Surface;
 import android.view.View;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import io.flutter.embedding.engine.renderer.FlutterRenderer;
 import io.flutter.embedding.engine.renderer.RenderSurface;
+import java.util.LinkedList;
+import java.util.Queue;
 
 /**
  * Paints a Flutter UI provided by an {@link android.media.ImageReader} onto a {@link
@@ -35,11 +37,10 @@
  * an {@link android.media.Image} and renders it to the {@link android.graphics.Canvas} in {@code
  * onDraw}.
  */
-@SuppressLint("ViewConstructor")
 @TargetApi(19)
 public class FlutterImageView extends View implements RenderSurface {
   @NonNull private ImageReader imageReader;
-  @Nullable private Image nextImage;
+  @Nullable private Queue<Image> imageQueue;
   @Nullable private Image currentImage;
   @Nullable private Bitmap currentBitmap;
   @Nullable private FlutterRenderer flutterRenderer;
@@ -70,17 +71,24 @@ public enum SurfaceKind {
    * the Flutter UI.
    */
   public FlutterImageView(@NonNull Context context, int width, int height, SurfaceKind kind) {
-    super(context, null);
-    this.imageReader = createImageReader(width, height);
-    this.kind = kind;
-    init();
+    this(context, createImageReader(width, height), kind);
+  }
+
+  public FlutterImageView(@NonNull Context context) {
+    this(context, 1, 1, SurfaceKind.background);
+  }
+
+  public FlutterImageView(@NonNull Context context, @NonNull AttributeSet attrs) {
+    this(context, 1, 1, SurfaceKind.background);
   }
 
   @VisibleForTesting
-  FlutterImageView(@NonNull Context context, @NonNull ImageReader imageReader, SurfaceKind kind) {
+  /*package*/ FlutterImageView(
+      @NonNull Context context, @NonNull ImageReader imageReader, SurfaceKind kind) {
     super(context, null);
     this.imageReader = imageReader;
     this.kind = kind;
+    this.imageQueue = new LinkedList<>();
     init();
   }
 
@@ -150,12 +158,14 @@ public void detachFromRenderer() {
     // attached to the renderer again.
     acquireLatestImage();
     // Clear drawings.
-    pendingImages = 0;
     currentBitmap = null;
-    if (nextImage != null) {
-      nextImage.close();
-      nextImage = null;
+
+    // Close the images in the queue and clear the queue.
+    for (final Image image : imageQueue) {
+      image.close();
     }
+    imageQueue.clear();
+    // Close and clear the current image if any.
     if (currentImage != null) {
       currentImage.close();
       currentImage = null;
@@ -168,7 +178,10 @@ public void pause() {
     // Not supported.
   }
 
-  /** Acquires the next image to be drawn to the {@link android.graphics.Canvas}. */
+  /**
+   * Acquires the next image to be drawn to the {@link android.graphics.Canvas}. Returns true if
+   * there's an image available in the queue.
+   */
   @TargetApi(19)
   public boolean acquireLatestImage() {
     if (!isAttachedToFlutterRenderer) {
@@ -182,14 +195,14 @@ public boolean acquireLatestImage() {
     // While the engine will also stop producing frames, there is a race condition.
     //
     // To avoid exceptions, check if a new image can be acquired.
-    if (pendingImages < imageReader.getMaxImages()) {
-      nextImage = imageReader.acquireLatestImage();
-      if (nextImage != null) {
-        pendingImages++;
+    if (imageQueue.size() < imageReader.getMaxImages()) {
+      final Image image = imageReader.acquireLatestImage();
+      if (image != null) {
+        imageQueue.add(image);
       }
     }
     invalidate();
-    return nextImage != null;
+    return !imageQueue.isEmpty();
   }
 
   /** Creates a new image reader with the provided size. */
@@ -200,15 +213,10 @@ public void resizeIfNeeded(int width, int height) {
     if (width == imageReader.getWidth() && height == imageReader.getHeight()) {
       return;
     }
-    // Close resources.
-    if (nextImage != null) {
-      nextImage.close();
-      nextImage = null;
-    }
-    if (currentImage != null) {
-      currentImage.close();
-      currentImage = null;
-    }
+    imageQueue.clear();
+    currentImage = null;
+    // Close all the resources associated with the image reader,
+    // including the images.
     imageReader.close();
     // Image readers cannot be resized once created.
     imageReader = createImageReader(width, height);
@@ -218,16 +226,14 @@ public void resizeIfNeeded(int width, int height) {
   @Override
   protected void onDraw(Canvas canvas) {
     super.onDraw(canvas);
-    if (nextImage != null) {
+
+    if (!imageQueue.isEmpty()) {
       if (currentImage != null) {
         currentImage.close();
-        pendingImages--;
       }
-      currentImage = nextImage;
-      nextImage = null;
+      currentImage = imageQueue.poll();
       updateCurrentBitmap();
     }
-
     if (currentBitmap != null) {
       canvas.drawBitmap(currentBitmap, 0, 0, null);
     }
@@ -238,6 +244,7 @@ private void updateCurrentBitmap() {
     if (android.os.Build.VERSION.SDK_INT >= 29) {
       final HardwareBuffer buffer = currentImage.getHardwareBuffer();
       currentBitmap = Bitmap.wrapHardwareBuffer(buffer, ColorSpace.get(ColorSpace.Named.SRGB));
+      buffer.close();
     } else {
       final Plane[] imagePlanes = currentImage.getPlanes();
       if (imagePlanes.length != 1) {
@@ -255,7 +262,6 @@ private void updateCurrentBitmap() {
             Bitmap.createBitmap(
                 desiredWidth, desiredHeight, android.graphics.Bitmap.Config.ARGB_8888);
       }
-
       currentBitmap.copyPixelsFromBuffer(imagePlane.getBuffer());
     }
   }

shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java

@@ -12,12 +12,15 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.content.res.Configuration;
 import android.content.res.Resources;
+import android.graphics.Canvas;
 import android.graphics.Insets;
 import android.media.Image;
+import android.media.Image.Plane;
 import android.media.ImageReader;
 import android.view.View;
 import android.view.ViewGroup;
@@ -550,6 +553,77 @@ public void flutterImageView_acquiresMaxImagesAtMost() {
     verify(mockReader, times(2)).acquireLatestImage();
   }
 
+  @Test
+  public void flutterImageView_detachFromRendererClosesAllImages() {
+    final ImageReader mockReader = mock(ImageReader.class);
+    when(mockReader.getMaxImages()).thenReturn(2);
+
+    final Image mockImage = mock(Image.class);
+    when(mockReader.acquireLatestImage()).thenReturn(mockImage);
+
+    final FlutterImageView imageView =
+        spy(
+            new FlutterImageView(
+                RuntimeEnvironment.application,
+                mockReader,
+                FlutterImageView.SurfaceKind.background));
+
+    final FlutterJNI jni = mock(FlutterJNI.class);
+    imageView.attachToRenderer(new FlutterRenderer(jni));
+
+    doNothing().when(imageView).invalidate();
+    imageView.acquireLatestImage();
+    imageView.acquireLatestImage();
+    imageView.detachFromRenderer();
+
+    verify(mockImage, times(2)).close();
+  }
+
+  @Test
+  @SuppressLint("WrongCall") /*View#onDraw*/
+  public void flutterImageView_onDrawClosesAllImages() {
+    final ImageReader mockReader = mock(ImageReader.class);
+    when(mockReader.getMaxImages()).thenReturn(2);
+
+    final Image mockImage = mock(Image.class);
+    when(mockImage.getPlanes()).thenReturn(new Plane[0]);
+    when(mockReader.acquireLatestImage()).thenReturn(mockImage);
+
+    final FlutterImageView imageView =
+        spy(
+            new FlutterImageView(
+                RuntimeEnvironment.application,
+                mockReader,
+                FlutterImageView.SurfaceKind.background));
+
+    final FlutterJNI jni = mock(FlutterJNI.class);
+    imageView.attachToRenderer(new FlutterRenderer(jni));
+
+    doNothing().when(imageView).invalidate();
+    imageView.acquireLatestImage();
+    imageView.acquireLatestImage();
+
+    imageView.onDraw(mock(Canvas.class));
+    imageView.onDraw(mock(Canvas.class));
+
+    // 1 image is closed and 1 is active.
+    verify(mockImage, times(1)).close();
+    verify(mockReader, times(2)).acquireLatestImage();
+
+    // This call doesn't do anything because there isn't
+    // an image in the queue.
+    imageView.onDraw(mock(Canvas.class));
+    verify(mockImage, times(1)).close();
+
+    // Aquire another image and push it to the queue.
+    imageView.acquireLatestImage();
+    verify(mockReader, times(3)).acquireLatestImage();
+
+    // Then, the second image is closed.
+    imageView.onDraw(mock(Canvas.class));
+    verify(mockImage, times(2)).close();
+  }
+
   /*
    * A custom shadow that reports fullscreen flag for system UI visibility
    */