Hook ImageReaderSurfaceProducer to the onTrimMemory listener interface

- Close all ImageReaders and Images when we get an onTrimMemory callback.
- Remove the first frame fix based around caching the last image displayed
  because it isn't safe to do on some platforms. Leave a TODO to revisit
  this.

Author: johnmccutchan

shell/platform/android/image_external_texture.cc

@@ -26,7 +26,6 @@ void ImageExternalTexture::Paint(PaintContext& context,
   if (state_ == AttachmentState::kDetached) {
     return;
   }
-  latest_bounds_ = bounds;
   Attach(context);
   const bool should_process_frame = !freeze;
   if (should_process_frame) {

shell/platform/android/image_external_texture.h

@@ -59,8 +59,6 @@ class ImageExternalTexture : public flutter::Texture {
 
   fml::jni::ScopedJavaGlobalRef<jobject> image_texture_entry_;
   std::shared_ptr<PlatformViewAndroidJNI> jni_facade_;
-  SkRect latest_bounds_ = SkRect::MakeEmpty();
-  fml::jni::ScopedJavaGlobalRef<jobject> latest_android_image_;
 
   enum class AttachmentState { kUninitialized, kAttached, kDetached };
   AttachmentState state_ = AttachmentState::kUninitialized;

shell/platform/android/image_external_texture_gl.cc

@@ -29,19 +29,19 @@ ImageExternalTextureGL::ImageExternalTextureGL(
 
 void ImageExternalTextureGL::Attach(PaintContext& context) {
   if (state_ == AttachmentState::kUninitialized) {
-    if (!latest_android_image_.is_null() && !latest_bounds_.isEmpty()) {
-      // After detach the cache of textures will have been cleared. If
-      // there is an android image we must populate it now so that the
-      // first frame isn't blank.
-      JavaLocalRef hardware_buffer = HardwareBufferFor(latest_android_image_);
-      UpdateImage(hardware_buffer, context);
-      CloseHardwareBuffer(hardware_buffer);
-    }
+    // TODO(johnmccurtchan): We currently display the first frame after an
+    // attach-detach cycle as blank. There seems to be an issue on some
+    // devices where ImageReaders/Images from before the detach aren't
+    // valid after the attach. According to Android folks this doesn't
+    // match the spec. Revisit this in the future.
+    // See https://github.com/flutter/flutter/issues/142978 and
+    // https://github.com/flutter/flutter/issues/139039.
     state_ = AttachmentState::kAttached;
   }
 }
 
 void ImageExternalTextureGL::UpdateImage(JavaLocalRef& hardware_buffer,
+                                         const SkRect& bounds,
                                          PaintContext& context) {
   AHardwareBuffer* latest_hardware_buffer = AHardwareBufferFor(hardware_buffer);
   std::optional<HardwareBufferKey> key =
@@ -57,7 +57,7 @@ void ImageExternalTextureGL::UpdateImage(JavaLocalRef& hardware_buffer,
     return;
   }
 
-  dl_image_ = CreateDlImage(context, latest_bounds_, key, std::move(egl_image));
+  dl_image_ = CreateDlImage(context, bounds, key, std::move(egl_image));
   if (key.has_value()) {
     gl_entries_.erase(image_lru_.AddImage(dl_image_, key.value()));
   }
@@ -70,17 +70,8 @@ void ImageExternalTextureGL::ProcessFrame(PaintContext& context,
     return;
   }
   JavaLocalRef hardware_buffer = HardwareBufferFor(image);
-  UpdateImage(hardware_buffer, context);
+  UpdateImage(hardware_buffer, bounds, context);
   CloseHardwareBuffer(hardware_buffer);
-
-  // NOTE: In the following code it is important that old_android_image is
-  // not closed until after the update of egl_image_ otherwise the image might
-  // be closed before the old EGLImage referencing it has been deleted. After
-  // an image is closed the underlying HardwareBuffer may be recycled and used
-  // for a future frame.
-  JavaLocalRef old_android_image(latest_android_image_);
-  latest_android_image_.Reset(image);
-  CloseImage(old_android_image);
 }
 
 void ImageExternalTextureGL::Detach() {

shell/platform/android/image_external_texture_gl.h

@@ -33,7 +33,9 @@ class ImageExternalTextureGL : public ImageExternalTexture {
   void Attach(PaintContext& context) override;
   void Detach() override;
   void ProcessFrame(PaintContext& context, const SkRect& bounds) override;
-  void UpdateImage(JavaLocalRef& hardware_buffer, PaintContext& context);
+  void UpdateImage(JavaLocalRef& hardware_buffer,
+                   const SkRect& bounds,
+                   PaintContext& context);
 
   virtual sk_sp<flutter::DlImage> CreateDlImage(
       PaintContext& context,

shell/platform/android/image_external_texture_vk.cc

@@ -38,9 +38,7 @@ void ImageExternalTextureVK::ProcessFrame(PaintContext& context,
   if (image.is_null()) {
     return;
   }
-  JavaLocalRef old_android_image(latest_android_image_);
-  latest_android_image_.Reset(image);
-  JavaLocalRef hardware_buffer = HardwareBufferFor(latest_android_image_);
+  JavaLocalRef hardware_buffer = HardwareBufferFor(image);
   AHardwareBuffer* latest_hardware_buffer = AHardwareBufferFor(hardware_buffer);
 
   AHardwareBuffer_Desc hb_desc = {};
@@ -53,9 +51,6 @@ void ImageExternalTextureVK::ProcessFrame(PaintContext& context,
     dl_image_ = existing_image;
 
     CloseHardwareBuffer(hardware_buffer);
-    // IMPORTANT: We have just received a new frame to display so close the
-    // previous Java Image so that it is recycled and used for a future frame.
-    CloseImage(old_android_image);
     return;
   }
 
@@ -105,9 +100,6 @@ void ImageExternalTextureVK::ProcessFrame(PaintContext& context,
     image_lru_.AddImage(dl_image_, key.value());
   }
   CloseHardwareBuffer(hardware_buffer);
-  // IMPORTANT: We have just received a new frame to display so close the
-  // previous Java Image so that it is recycled and used for a future frame.
-  CloseImage(old_android_image);
 }
 
 }  // namespace flutter

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

@@ -189,6 +189,7 @@ public SurfaceProducer createSurfaceProducer() {
     if (!debugForceSurfaceProducerGlTextures && Build.VERSION.SDK_INT >= 29) {
       final ImageReaderSurfaceProducer producer = new ImageReaderSurfaceProducer(id);
       registerImageTexture(id, producer);
+      addOnTrimMemoryListener(producer);
       Log.v(TAG, "New ImageReaderSurfaceProducer ID: " + id);
       entry = producer;
     } else {
@@ -404,7 +405,9 @@ public void run() {
   @Keep
   @TargetApi(29)
   final class ImageReaderSurfaceProducer
-      implements TextureRegistry.SurfaceProducer, TextureRegistry.ImageConsumer {
+      implements TextureRegistry.SurfaceProducer,
+          TextureRegistry.ImageConsumer,
+          TextureRegistry.OnTrimMemoryListener {
     private static final String TAG = "ImageReaderSurfaceProducer";
     private static final int MAX_IMAGES = 4;
 
@@ -436,6 +439,7 @@ final class ImageReaderSurfaceProducer
     private final LinkedList<PerImageReader> imageReaderQueue = new LinkedList<PerImageReader>();
     private final HashMap<ImageReader, PerImageReader> perImageReaders =
         new HashMap<ImageReader, PerImageReader>();
+    private PerImage lastDequeuedImage = null;
     private PerImageReader lastReaderDequeuedFrom = null;
 
     /** Internal class: state held per Image produced by ImageReaders. */
@@ -607,7 +611,15 @@ PerImage dequeueImage() {
               lastDequeueTime = System.nanoTime();
             }
           }
-          // We have dequeued the first image from reader.
+          if (lastDequeuedImage != null) {
+            // We must keep the last image dequeued open until we are done presenting it.
+            // We have just dequeued a new image (r). Close the previously dequeued image.
+            lastDequeuedImage.image.close();
+            lastDequeuedImage = null;
+          }
+          // Remember the last image and reader dequeued from. We do this because we must
+          // keep both of these alive until we are done presenting the image.
+          lastDequeuedImage = r;
           lastReaderDequeuedFrom = reader;
           break;
         }
@@ -616,23 +628,40 @@ PerImage dequeueImage() {
       return r;
     }
 
+    @Override
+    public void onTrimMemory(int level) {
+      cleanup();
+      createNewReader = true;
+    }
+
     private void releaseInternal() {
+      cleanup();
       released = true;
-      for (PerImageReader pir : perImageReaders.values()) {
-        pir.close();
+    }
+
+    private void cleanup() {
+      synchronized (lock) {
+        for (PerImageReader pir : perImageReaders.values()) {
+          pir.close();
+        }
+        perImageReaders.clear();
+        if (lastDequeuedImage != null) {
+          lastDequeuedImage.image.close();
+          lastDequeuedImage = null;
+        }
+        if (lastReaderDequeuedFrom != null) {
+          lastReaderDequeuedFrom.close();
+          lastReaderDequeuedFrom = null;
+        }
+        imageReaderQueue.clear();
       }
-      perImageReaders.clear();
-      imageReaderQueue.clear();
     }
 
     @TargetApi(33)
     private void waitOnFence(Image image) {
       try {
         SyncFence fence = image.getFence();
-        boolean signaled = fence.awaitForever();
-        if (!signaled) {
-          Log.e(TAG, "acquireLatestImage image's fence was never signalled.");
-        }
+        fence.awaitForever();
       } catch (IOException e) {
         // Drop.
       }
@@ -874,10 +903,7 @@ public void pushImage(Image image) {
     private void waitOnFence(Image image) {
       try {
         SyncFence fence = image.getFence();
-        boolean signaled = fence.awaitForever();
-        if (!signaled) {
-          Log.e(TAG, "acquireLatestImage image's fence was never signalled.");
-        }
+        fence.awaitForever();
       } catch (IOException e) {
         // Drop.
       }

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

@@ -622,6 +622,40 @@ public void ImageReaderSurfaceProducerImageReadersAndImagesCount() {
     assertEquals(0, texture.numImages());
   }
 
+  @Test
+  public void ImageReaderSurfaceProducerTrimMemoryCallback() {
+    FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
+    FlutterRenderer.ImageReaderSurfaceProducer texture =
+        flutterRenderer.new ImageReaderSurfaceProducer(0);
+    texture.disableFenceForTest();
+
+    // Returns a null image when one hasn't been produced.
+    assertNull(texture.acquireLatestImage());
+
+    // Give the texture an initial size.
+    texture.setSize(1, 1);
+
+    // Grab the surface so we can render a frame at 1x1 after resizing.
+    Surface surface = texture.getSurface();
+    assertNotNull(surface);
+    Canvas canvas = surface.lockHardwareCanvas();
+    canvas.drawARGB(255, 255, 0, 0);
+    surface.unlockCanvasAndPost(canvas);
+
+    // Let callbacks run, this will produce a single frame.
+    shadowOf(Looper.getMainLooper()).idle();
+
+    assertEquals(1, texture.numImageReaders());
+    assertEquals(1, texture.numImages());
+
+    // Invoke the onTrimMemory callback.
+    texture.onTrimMemory(0);
+    shadowOf(Looper.getMainLooper()).idle();
+
+    assertEquals(0, texture.numImageReaders());
+    assertEquals(0, texture.numImages());
+  }
+
   // A 0x0 ImageReader is a runtime error.
   @Test
   public void ImageReaderSurfaceProducerClampsWidthAndHeightTo1() {