Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix memory leak in PlatformViewsController #27915

Merged
merged 3 commits into from
Aug 9, 2021
Merged

Fix memory leak in PlatformViewsController #27915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
557f3e6
Fix memory leak in PlatformViewsController
Aug 5, 2021
903846d
Fix test
Aug 5, 2021
207d335
Revert run_tests.py
Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega
private View flutterView;

// The texture registry maintaining the textures into which the embedded views will be rendered.
private TextureRegistry textureRegistry;
@Nullable private TextureRegistry textureRegistry;

@Nullable private TextInputPlugin textInputPlugin;

Expand All @@ -79,7 +79,7 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega
// Since each virtual display has it's unique context this allows associating any view with the
// platform view that
// it is associated with(e.g if a platform view creates other views in the same virtual display.
private final HashMap<Context, View> contextToPlatformView;
@VisibleForTesting /* package */ final HashMap<Context, View> contextToPlatformView;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is /* package */ necessary? This should be the implied visibility right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it increases readability since the Java language isn't very descriptive. Debatable, but it's commonly used in g3 as well as in the engine.


// The views returned by `PlatformView#getView()`.
//
Expand Down Expand Up @@ -711,6 +711,10 @@ private void flushAllViews() {
while (platformViews.size() > 0) {
channelHandler.disposeAndroidViewForPlatformView(platformViews.keyAt(0));
}

if (contextToPlatformView.size() > 0) {
contextToPlatformView.clear();
}
}

private void initializeRootImageViewIfNeeded() {
Expand Down
120 changes: 105 additions & 15 deletions shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@

import android.content.Context;
import android.content.res.AssetManager;
import android.graphics.SurfaceTexture;
import android.util.SparseArray;
import android.view.MotionEvent;
import android.view.Surface;
import android.view.SurfaceHolder;
import android.view.SurfaceView;
import android.view.View;
import android.view.ViewParent;
import android.widget.FrameLayout.LayoutParams;
import io.flutter.embedding.android.FlutterImageView;
import io.flutter.embedding.android.FlutterView;
import io.flutter.embedding.android.MotionEventTracker;
Expand All @@ -32,6 +34,7 @@
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.StandardMethodCodec;
import io.flutter.plugin.localization.LocalizationPlugin;
import io.flutter.view.TextureRegistry;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -232,7 +235,7 @@ public void getPlatformViewById__hybridComposition() {
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);

platformViewsController.initializePlatformViewIfNeeded(platformViewId);

Expand All @@ -259,7 +262,7 @@ public void createPlatformViewMessage__initializesAndroidView() {
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);
verify(viewFactory, times(1)).create(any(), eq(platformViewId), any());
}

Expand All @@ -281,7 +284,7 @@ public void createPlatformViewMessage__throwsIfViewIsNull() {
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);
assertEquals(ShadowFlutterJNI.getResponses().size(), 1);

assertThrows(
Expand All @@ -291,6 +294,66 @@ public void createPlatformViewMessage__throwsIfViewIsNull() {
});
}

@Test
@Config(shadows = {ShadowFlutterJNI.class})
public void onDetachedFromJNI_clearsPlatformViewContext() {
PlatformViewsController platformViewsController = new PlatformViewsController();

int platformViewId = 0;
assertNull(platformViewsController.getPlatformViewById(platformViewId));

PlatformViewFactory viewFactory = mock(PlatformViewFactory.class);
PlatformView platformView = mock(PlatformView.class);

View pv = mock(View.class);
when(pv.getLayoutParams()).thenReturn(new LayoutParams(1, 1));

when(platformView.getView()).thenReturn(pv);
when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView);
platformViewsController.getRegistry().registerViewFactory("testType", viewFactory);

FlutterJNI jni = new FlutterJNI();
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(
jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ false);

assertFalse(platformViewsController.contextToPlatformView.isEmpty());
platformViewsController.onDetachedFromJNI();
assertTrue(platformViewsController.contextToPlatformView.isEmpty());
}

@Test
@Config(shadows = {ShadowFlutterJNI.class})
public void onPreEngineRestart_clearsPlatformViewContext() {
PlatformViewsController platformViewsController = new PlatformViewsController();

int platformViewId = 0;
assertNull(platformViewsController.getPlatformViewById(platformViewId));

PlatformViewFactory viewFactory = mock(PlatformViewFactory.class);
PlatformView platformView = mock(PlatformView.class);

View pv = mock(View.class);
when(pv.getLayoutParams()).thenReturn(new LayoutParams(1, 1));

when(platformView.getView()).thenReturn(pv);
when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView);
platformViewsController.getRegistry().registerViewFactory("testType", viewFactory);

FlutterJNI jni = new FlutterJNI();
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(
jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ false);

assertFalse(platformViewsController.contextToPlatformView.isEmpty());
platformViewsController.onDetachedFromJNI();
assertTrue(platformViewsController.contextToPlatformView.isEmpty());
}

@Test
@Config(shadows = {ShadowFlutterJNI.class})
public void createPlatformViewMessage__throwsIfViewHasParent() {
Expand All @@ -311,7 +374,7 @@ public void createPlatformViewMessage__throwsIfViewHasParent() {
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);
assertEquals(ShadowFlutterJNI.getResponses().size(), 1);

assertThrows(
Expand Down Expand Up @@ -343,7 +406,7 @@ public void disposeAndroidView__hybridComposition() {
attach(jni, platformViewsController);

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);
platformViewsController.initializePlatformViewIfNeeded(platformViewId);

assertNotNull(androidView.getParent());
Expand All @@ -354,7 +417,7 @@ public void disposeAndroidView__hybridComposition() {
assertNull(androidView.getParent());

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);
platformViewsController.initializePlatformViewIfNeeded(platformViewId);

assertNotNull(androidView.getParent());
Expand Down Expand Up @@ -384,7 +447,7 @@ public void onEndFrame__destroysOverlaySurfaceAfterFrameOnFlutterSurfaceView() {
jni.onFirstFrame();

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);

// Produce a frame that displays a platform view and an overlay surface.
platformViewsController.onBeginFrame();
Expand Down Expand Up @@ -447,7 +510,7 @@ public void onEndFrame__removesPlatformView() {
jni.onFirstFrame();

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);

// Simulate first frame from the framework.
jni.onFirstFrame();
Expand Down Expand Up @@ -484,7 +547,7 @@ public void onEndFrame__removesPlatformViewParent() {
jni.onFirstFrame();

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);
platformViewsController.initializePlatformViewIfNeeded(platformViewId);
assertEquals(flutterView.getChildCount(), 2);

Expand Down Expand Up @@ -520,7 +583,7 @@ public void detach__destroysOverlaySurfaces() {
jni.onFirstFrame();

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);

// Produce a frame that displays a platform view and an overlay surface.
platformViewsController.onBeginFrame();
Expand Down Expand Up @@ -653,7 +716,7 @@ public void convertPlatformViewRenderSurfaceAsDefault() {
jni.onFirstFrame();

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);

// Produce a frame that displays a platform view and an overlay surface.
platformViewsController.onBeginFrame();
Expand Down Expand Up @@ -702,7 +765,7 @@ public void dontConverRenderSurfaceWhenFlagIsTrue() {
synchronizeToNativeViewHierarchy(jni, platformViewsController, false);

// Simulate create call from the framework.
createPlatformView(jni, platformViewsController, platformViewId, "testType");
createPlatformView(jni, platformViewsController, platformViewId, "testType", /* hybrid=*/ true);

// Produce a frame that displays a platform view and an overlay surface.
platformViewsController.onBeginFrame();
Expand Down Expand Up @@ -734,12 +797,16 @@ private static void createPlatformView(
FlutterJNI jni,
PlatformViewsController platformViewsController,
int platformViewId,
String viewType) {
String viewType,
boolean hybrid) {
final Map<String, Object> platformViewCreateArguments = new HashMap<>();
platformViewCreateArguments.put("hybrid", true);
platformViewCreateArguments.put("hybrid", hybrid);
platformViewCreateArguments.put("id", platformViewId);
platformViewCreateArguments.put("viewType", viewType);
platformViewCreateArguments.put("direction", 0);
platformViewCreateArguments.put("width", 1.0);
platformViewCreateArguments.put("height", 1.0);

final MethodCall platformCreateMethodCall =
new MethodCall("create", platformViewCreateArguments);

Expand Down Expand Up @@ -776,7 +843,30 @@ private static FlutterView attach(
executor.onAttachedToJNI();

final Context context = RuntimeEnvironment.application.getApplicationContext();
platformViewsController.attach(context, null, executor);
final TextureRegistry registry =
new TextureRegistry() {
public void TextureRegistry() {}

@Override
public SurfaceTextureEntry createSurfaceTexture() {
return new SurfaceTextureEntry() {
@Override
public SurfaceTexture surfaceTexture() {
return mock(SurfaceTexture.class);
}

@Override
public long id() {
return 0;
}

@Override
public void release() {}
};
}
};

platformViewsController.attach(context, registry, executor);

final FlutterView view =
new FlutterView(context, FlutterView.RenderMode.surface) {
Expand Down