firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add SurfaceProducer.onSurfaceCleanup, deprecate onSurfaceDestroyed. (#160937)

Browse Source 
Closes https://github.com/flutter/flutter/issues/160933.

The timing of this callback gives our users (and plugin authors) a
chance to stop using the `Surface` before it becomes invalid, allowing
us to fix https://github.com/flutter/flutter/issues/156488 - we no
longer need to do shenanigans on storing and restoring state because
`ExoPlayer` can now handle it out of the box; see
https://github.com/flutter/flutter/issues/160933#issuecomment-2564092567.

It's unfortunate we have to go through a bit of churn on the callback
API, but realistically this _is_ the feedback we were looking for when
originally creating it - it just took longer than expected due to the
long release cycle.

/cc @hasali19, @xxoo, @camsim99
This commit is contained in:
Matan Lurey 2025-01-06 10:03:11 -08:00 committed by GitHub
parent 6d729b6d1d
commit 351f2742af
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 67 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java
View File

@ -464,7 +464,7 @@ public class FlutterRenderer implements TextureRegistry {
private boolean createNewReader = true;
/**
* Stores whether {@link Callback#onSurfaceDestroyed()} was previously invoked.
* Stores whether {@link Callback#onSurfaceCleanup()} ()} was previously invoked.
*
* <p>Used to avoid signaling {@link Callback#onSurfaceAvailable()} unnecessarily.
*/
@ -704,6 +704,7 @@ public class FlutterRenderer implements TextureRegistry {
}
@Override
@SuppressWarnings({"deprecation", "removal"})
public void onTrimMemory(int level) {
if (!trimOnMemoryPressure) {
return;
@ -714,6 +715,10 @@ public class FlutterRenderer implements TextureRegistry {
synchronized (lock) {
numTrims++;
}
if (this.callback != null) {
notifiedDestroy = true;
this.callback.onSurfaceCleanup();
}
cleanup();
createNewReader = true;
if (this.callback != null) {

21
engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java
View File

@ -145,6 +145,23 @@ public interface TextureRegistry {
*
* <p>In a low memory environment, the Android OS will signal to Flutter to release resources,
* such as surfaces, that are not currently in use, such as when the application is in the
* background, and this method is subsequently called to notify a plugin author to stop using
* or rendering to the last surface.
*
* @deprecated Override and use {@link Callback#onSurfaceCleanup()} instead. This method is
* called after the surface has already been destroyed, which is often too late to tell a
* dependency (which might have already scheduled a render) to stop.
* @see <a href="https://github.com/flutter/flutter/issues/160933">#160933</a>.
*/
@Deprecated(since = "Flutter 3.28", forRemoval = true)
default void onSurfaceDestroyed() {}
/**
* Invoked when a {@link Surface} returned by {@link SurfaceProducer#getSurface()} is about
* to become invalid.
*
* <p>In a low memory environment, the Android OS will signal to Flutter to release resources,
* such as surfaces, that are not currently in use, such as when the application is in the
* background, and this method is subsequently called to notify a plugin author to stop
* using or rendering to the last surface.
*
@ -155,7 +172,7 @@ public interface TextureRegistry {
* void example(SurfaceProducer producer) {
* producer.setCallback(new SurfaceProducer.Callback() {
* @override
* public void onSurfaceDestroyed() {
* public void onSurfaceCleanup() {
* // Store information about the last frame, if necessary.
* // Potentially release other dependent resources.
* }
@ -166,7 +183,7 @@ public interface TextureRegistry {
* }
* </pre>
*/
void onSurfaceDestroyed();
default void onSurfaceCleanup() {}
}
/** This method is not officially part of the public API surface and will be deprecated. */

42
engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java
View File

@ -766,6 +766,7 @@ public class FlutterRendererTest {
}
@Test
@SuppressWarnings({"deprecation", "removal"})
public void ImageReaderSurfaceProducerIsDestroyedOnTrimMemory() {
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer();
@ -779,10 +780,49 @@ public class FlutterRendererTest {
flutterRenderer.onTrimMemory(TRIM_MEMORY_BACKGROUND);
// Verify.
verify(callback).onSurfaceCleanup();
verify(callback).onSurfaceDestroyed();
}
private static class TestSurfaceState {
Surface beingDestroyed;
}
@Test
public void ImageReaderSurfaceProducerSignalsCleanupBeforeDestroying() throws Exception {
// Regression test for https://github.com/flutter/flutter/issues/160933.
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer();
// Ensure the callbacks were actually called.
// Note this needs to be an object in order to be accessed in the callback.
final TestSurfaceState state = new TestSurfaceState();
state.beingDestroyed = producer.getSurface();
// Create and set a callback that ensures the surface is not yet released.
CountDownLatch latch = new CountDownLatch(1);
producer.setCallback(
new TextureRegistry.SurfaceProducer.Callback() {
@Override
public void onSurfaceCleanup() {
state.beingDestroyed = producer.getSurface();
assertTrue("Not released yet", state.beingDestroyed.isValid());
state.beingDestroyed.release();
latch.countDown();
}
});
// Trim.
flutterRenderer.onTrimMemory(TRIM_MEMORY_BACKGROUND);
latch.await();
// Destroy.
assertFalse("Should be destroyed", state.beingDestroyed.isValid());
}
@Test
@SuppressWarnings({"deprecation", "removal"})
public void ImageReaderSurfaceProducerUnsubscribesWhenReleased() {
// Regression test for https://github.com/flutter/flutter/issues/156434.
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
@ -800,10 +840,12 @@ public class FlutterRendererTest {
flutterRenderer.onTrimMemory(TRIM_MEMORY_BACKGROUND);
// Verify was not called.
verify(callback, never()).onSurfaceCleanup();
verify(callback, never()).onSurfaceDestroyed();
}
@Test
@SuppressWarnings({"deprecation", "removal"})
public void ImageReaderSurfaceProducerIsCreatedOnLifecycleResume() throws Exception {
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer();