Skip to content

Commit 9467677

Browse files
authored
Change timing of onSurfaceDestroyed to match onSurfaceCleanup (#161252)
Follow-up to flutter/flutter#160937 (flutter/flutter#160933). This more or less mirrors what we did already for `onSurfaceCreated` to match `onSurfaceAvailable`. With this approach, the master branch should immediately start seeing better behavior in terms of being able to use the `onSurfaceDestroyed` callback effectively (before the surface has been released). For example, for a plugin that already uses `onSurfaceDestroyed`, [i.e `camerax`](https://github.com/flutter/packages/blob/97ce56a68eea650dc784617b4eed7814cccedeb8/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/PreviewHostApiImpl.java): ```java @OverRide public void onSurfaceDestroyed() { // Invalidate the SurfaceRequest so that CameraX knows to to make a new request // for a surface. request.invalidate(); } ``` ... the request is now invalidated _before_ the surface has been destroyed, which is what (I believe) we wanted? --- Folks that want to publish package updates that _definitely_ use the correct timing will have to wait for the next stable. /cc @hasali19 would be great to get your input here.
1 parent 13ae4cd commit 9467677

File tree

3 files changed

+27
-17
lines changed

3 files changed

+27
-17
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -721,10 +721,6 @@ public void onTrimMemory(int level) {
721721
}
722722
cleanup();
723723
createNewReader = true;
724-
if (this.callback != null) {
725-
notifiedDestroy = true;
726-
this.callback.onSurfaceDestroyed();
727-
}
728724
}
729725

730726
private void releaseInternal() {

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ default void onSurfaceCreated() {}
116116
/**
117117
* Invoked when an Android application is resumed after {@link Callback#onSurfaceDestroyed()}.
118118
*
119+
* <p>When this method is overridden, {@link Callback#onSurfaceCreated()} is not called.
120+
*
119121
* <p>Applications should now call {@link SurfaceProducer#getSurface()} to get a new
120122
* {@link Surface}, as the previous one was destroyed and released as a result of a low memory
121123
* event from the Android OS.
@@ -141,17 +143,9 @@ default void onSurfaceAvailable() {
141143
}
142144

143145
/**
144-
* Invoked when a {@link Surface} returned by {@link SurfaceProducer#getSurface()} is invalid.
145-
*
146-
* <p>In a low memory environment, the Android OS will signal to Flutter to release resources,
147-
* such as surfaces, that are not currently in use, such as when the application is in the
148-
* background, and this method is subsequently called to notify a plugin author to stop using
149-
* or rendering to the last surface.
146+
* An alias for {@link Callback#onSurfaceCleanup()} with a less accurate name.
150147
*
151-
* @deprecated Override and use {@link Callback#onSurfaceCleanup()} instead. This method is
152-
* called after the surface has already been destroyed, which is often too late to tell a
153-
* dependency (which might have already scheduled a render) to stop.
154-
* @see <a href="https://github.com/flutter/flutter/issues/160933">#160933</a>.
148+
* @deprecated Override and use {@link Callback#onSurfaceCleanup()} instead.
155149
*/
156150
@Deprecated(since = "Flutter 3.28", forRemoval = true)
157151
default void onSurfaceDestroyed() {}
@@ -160,6 +154,8 @@ default void onSurfaceDestroyed() {}
160154
* Invoked when a {@link Surface} returned by {@link SurfaceProducer#getSurface()} is about
161155
* to become invalid.
162156
*
157+
* <p>When this method is overridden, {@link Callback#onSurfaceDestroyed()} is not called.
158+
*
163159
* <p>In a low memory environment, the Android OS will signal to Flutter to release resources,
164160
* such as surfaces, that are not currently in use, such as when the application is in the
165161
* background, and this method is subsequently called to notify a plugin author to stop
@@ -183,7 +179,9 @@ default void onSurfaceDestroyed() {}
183179
* }
184180
* </pre>
185181
*/
186-
default void onSurfaceCleanup() {}
182+
default void onSurfaceCleanup() {
183+
onSurfaceDestroyed();
184+
}
187185
}
188186

189187
/** This method is not officially part of the public API surface and will be deprecated. */

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ public void ImageReaderSurfaceProducerDoesNotCropOrRotate() {
767767

768768
@Test
769769
@SuppressWarnings({"deprecation", "removal"})
770-
public void ImageReaderSurfaceProducerIsDestroyedOnTrimMemory() {
770+
public void ImageReaderSurfaceProducerIsCleanedUpOnTrimMemory() {
771771
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
772772
TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer();
773773

@@ -781,7 +781,6 @@ public void ImageReaderSurfaceProducerIsDestroyedOnTrimMemory() {
781781

782782
// Verify.
783783
verify(callback).onSurfaceCleanup();
784-
verify(callback).onSurfaceDestroyed();
785784
}
786785

787786
private static class TestSurfaceState {
@@ -821,6 +820,23 @@ public void onSurfaceCleanup() {
821820
assertFalse("Should be destroyed", state.beingDestroyed.isValid());
822821
}
823822

823+
@Test
824+
@SuppressWarnings({"deprecation", "removal"})
825+
public void ImageReaderSurfaceProducerSignalsCleanupCallsDestroy() throws Exception {
826+
CountDownLatch latch = new CountDownLatch(1);
827+
TextureRegistry.SurfaceProducer.Callback callback =
828+
new TextureRegistry.SurfaceProducer.Callback() {
829+
@Override
830+
public void onSurfaceDestroyed() {
831+
latch.countDown();
832+
}
833+
};
834+
835+
// Tests that cleanup, if not provided, just calls destroyed.
836+
callback.onSurfaceCleanup();
837+
latch.await();
838+
}
839+
824840
@Test
825841
@SuppressWarnings({"deprecation", "removal"})
826842
public void ImageReaderSurfaceProducerUnsubscribesWhenReleased() {

0 commit comments

Comments
 (0)