Skip to content

Commit 0353165

Browse files
[CP-stable][Android] Refactor ImageReaderSurfaceProducer restoration after app resumes (flutter#177112)
> [!NOTE] > To release engineer: this can definitely wait to be included in a hotfix beta release. I also want to CP this to 3.38 beta as part of a hotfix release: flutter#177121 This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? flutter#173770 ### Changelog Description: Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples Mitigates a memory leak that occurs on Android, when `Activities` are not kept upon exit and an Activity is exited and re-entered. ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch) Memory leak that will occur in production app but does not impact development. ### Workaround: Is there a workaround for this issue? Not that I know of. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? Follow the steps in the [issue description](flutter#173770 (comment)), roughly: 1. Create a Flutter project. 2. Enter the developer options, set not to keep activities, (destroy each activity as soon as the user leaves it) 3. Entering the activity, clicking the home button, exiting the activity, and then entering the activity again will trigger the destruction and reconstruction of the activity. 4. Check memory leaks through profiler and find memory leaks.
1 parent 9f455d2 commit 0353165

File tree

4 files changed

+51
-55
lines changed

4 files changed

+51
-55
lines changed

engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,7 @@ public boolean onPreDraw() {
571571
void onResume() {
572572
Log.v(TAG, "onResume()");
573573
ensureAlive();
574+
flutterEngine.getRenderer().restoreSurfaceProducers();
574575
if (host.shouldDispatchAppLifecycleState() && flutterEngine != null) {
575576
flutterEngine.getLifecycleChannel().appIsResumed();
576577
}

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

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@
2424
import androidx.annotation.Nullable;
2525
import androidx.annotation.RequiresApi;
2626
import androidx.annotation.VisibleForTesting;
27-
import androidx.lifecycle.DefaultLifecycleObserver;
28-
import androidx.lifecycle.LifecycleOwner;
29-
import androidx.lifecycle.ProcessLifecycleOwner;
3027
import io.flutter.Log;
3128
import io.flutter.embedding.engine.FlutterJNI;
3229
import io.flutter.view.TextureRegistry;
@@ -100,21 +97,23 @@ public void onFlutterUiNoLongerDisplayed() {
10097
public FlutterRenderer(@NonNull FlutterJNI flutterJNI) {
10198
this.flutterJNI = flutterJNI;
10299
this.flutterJNI.addIsDisplayingFlutterUiListener(flutterUiDisplayListener);
103-
ProcessLifecycleOwner.get()
104-
.getLifecycle()
105-
.addObserver(
106-
new DefaultLifecycleObserver() {
107-
@Override
108-
public void onResume(@NonNull LifecycleOwner owner) {
109-
Log.v(TAG, "onResume called; notifying SurfaceProducers");
110-
for (ImageReaderSurfaceProducer producer : imageReaderProducers) {
111-
if (producer.callback != null && producer.notifiedDestroy) {
112-
producer.notifiedDestroy = false;
113-
producer.callback.onSurfaceAvailable();
114-
}
115-
}
116-
}
117-
});
100+
}
101+
102+
/**
103+
* Restores {@code ImageReaderSurfaceProducer}s that were previously notified to be destroyed due
104+
* to a call to {@code onTrimMemory} as part of an {@code onResume} app lifecycle event.
105+
*
106+
* <p>All {@code FlutterActivity}s and {@code FlutterFragment}s are expected to call this {@code
107+
* onResume} to ensure surface producers are restored when the app returns to the foreground.
108+
*/
109+
public void restoreSurfaceProducers() {
110+
Log.v(TAG, "restoreSurfaceProducers called; notifying SurfaceProducers");
111+
for (ImageReaderSurfaceProducer producer : imageReaderProducers) {
112+
if (producer.callback != null && producer.notifiedDestroy) {
113+
producer.notifiedDestroy = false;
114+
producer.callback.onSurfaceAvailable();
115+
}
116+
}
118117
}
119118

120119
/**
@@ -452,7 +451,7 @@ final class ImageReaderSurfaceProducer
452451
*
453452
* <p>Used to avoid signaling {@link Callback#onSurfaceAvailable()} unnecessarily.
454453
*/
455-
private boolean notifiedDestroy = false;
454+
@VisibleForTesting boolean notifiedDestroy = false;
456455

457456
// State held to track latency of various stages.
458457
private long lastDequeueTime = 0;
@@ -466,7 +465,8 @@ final class ImageReaderSurfaceProducer
466465
private final HashMap<ImageReader, PerImageReader> perImageReaders = new HashMap<>();
467466
private ArrayList<PerImage> lastDequeuedImage = new ArrayList<PerImage>();
468467
private PerImageReader lastReaderDequeuedFrom = null;
469-
private Callback callback = null;
468+
469+
@VisibleForTesting Callback callback = null;
470470

471471
/** Internal class: state held per Image produced by ImageReaders. */
472472
private class PerImage {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,16 @@ public void itDoesNotDetachTwice() {
16131613
}
16141614
}
16151615

1616+
@Test
1617+
public void onResume_restoresSurfaceProducers() {
1618+
FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
1619+
1620+
delegate.onAttach(ctx);
1621+
delegate.onResume();
1622+
1623+
verify(mockFlutterEngine.getRenderer()).restoreSurfaceProducers();
1624+
}
1625+
16161626
/**
16171627
* Creates a mock {@link io.flutter.embedding.engine.FlutterEngine}.
16181628
*

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

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@
3232
import android.media.ImageReader;
3333
import android.os.Looper;
3434
import android.view.Surface;
35-
import androidx.lifecycle.Lifecycle;
36-
import androidx.lifecycle.LifecycleRegistry;
37-
import androidx.lifecycle.ProcessLifecycleOwner;
3835
import androidx.test.ext.junit.rules.ActivityScenarioRule;
3936
import androidx.test.ext.junit.runners.AndroidJUnit4;
4037
import io.flutter.embedding.android.FlutterActivity;
@@ -924,38 +921,6 @@ public void ImageReaderSurfaceProducerUnsubscribesWhenReleased() {
924921
verify(callback, never()).onSurfaceDestroyed();
925922
}
926923

927-
@Test
928-
@SuppressWarnings({"deprecation", "removal"})
929-
public void ImageReaderSurfaceProducerIsCreatedOnLifecycleResume() throws Exception {
930-
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
931-
TextureRegistry.SurfaceProducer producer =
932-
flutterRenderer.createSurfaceProducer(TextureRegistry.SurfaceLifecycle.resetInBackground);
933-
934-
// Create a callback.
935-
CountDownLatch latch = new CountDownLatch(1);
936-
TextureRegistry.SurfaceProducer.Callback callback =
937-
new TextureRegistry.SurfaceProducer.Callback() {
938-
@Override
939-
public void onSurfaceAvailable() {
940-
latch.countDown();
941-
}
942-
943-
@Override
944-
public void onSurfaceDestroyed() {}
945-
};
946-
producer.setCallback(callback);
947-
948-
// Trim memory.
949-
flutterRenderer.onTrimMemory(TRIM_MEMORY_BACKGROUND);
950-
951-
// Trigger a resume.
952-
((LifecycleRegistry) ProcessLifecycleOwner.get().getLifecycle())
953-
.setCurrentState(Lifecycle.State.RESUMED);
954-
955-
// Verify.
956-
latch.await();
957-
}
958-
959924
@Test
960925
public void ImageReaderSurfaceProducerSchedulesFrameIfQueueNotEmpty() throws Exception {
961926
FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer());
@@ -1062,4 +1027,24 @@ public void getSurface_doesNotReturnNewSurface() {
10621027

10631028
assertEquals(firstSurface, secondSurface);
10641029
}
1030+
1031+
@Test
1032+
public void restoreSurfaceProducers_restoresImageReaderSurfaceProducersAsIfApplicationResumed() {
1033+
FlutterRenderer flutterRenderer = engineRule.getFlutterEngine().getRenderer();
1034+
FlutterRenderer.ImageReaderSurfaceProducer imageReaderProducer1 =
1035+
(FlutterRenderer.ImageReaderSurfaceProducer) flutterRenderer.createSurfaceProducer();
1036+
FlutterRenderer.ImageReaderSurfaceProducer imageReaderProducer2 =
1037+
(FlutterRenderer.ImageReaderSurfaceProducer) flutterRenderer.createSurfaceProducer();
1038+
imageReaderProducer1.callback = mock(TextureRegistry.SurfaceProducer.Callback.class);
1039+
imageReaderProducer2.callback = mock(TextureRegistry.SurfaceProducer.Callback.class);
1040+
imageReaderProducer1.notifiedDestroy = true;
1041+
imageReaderProducer2.notifiedDestroy = true;
1042+
1043+
flutterRenderer.restoreSurfaceProducers();
1044+
1045+
verify(imageReaderProducer1.callback).onSurfaceAvailable();
1046+
verify(imageReaderProducer2.callback).onSurfaceAvailable();
1047+
assertFalse(imageReaderProducer1.notifiedDestroy);
1048+
assertFalse(imageReaderProducer2.notifiedDestroy);
1049+
}
10651050
}

0 commit comments

Comments
 (0)