Skip to content

Commit

Permalink
Eagerly remove the PlatformView from the view hierarchy on Android (f…
Browse files Browse the repository at this point in the history
…lutter#43423)

Eagerly remove the PlatformView from the view hierarchy on Android.

Fixes [#107297](flutter/flutter#107297)

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.
  • Loading branch information
johnmccutchan authored and kjlubick committed Jul 14, 2023
1 parent 1e10dae commit 69c76a4
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,22 @@ public void dispose(int viewId) {
Log.e(TAG, "Disposing unknown platform view with id: " + viewId);
return;
}
if (platformView.getView() != null) {
final View embeddedView = platformView.getView();
final ViewGroup pvParent = (ViewGroup) embeddedView.getParent();
if (pvParent != null) {
// Eagerly remove the embedded view from the PlatformViewWrapper.
// Without this call, we see some crashes because removing the view
// is used as a signal to stop processing.
pvParent.removeView(embeddedView);
}
}
platformViews.remove(viewId);

try {
platformView.dispose();
} catch (RuntimeException exception) {
Log.e(TAG, "Disposing platform view threw an exception", exception);
}

if (usesVirtualDisplay(viewId)) {
final VirtualDisplayController vdController = vdControllers.get(viewId);
final View embeddedView = vdController.getView();
Expand Down Expand Up @@ -581,7 +589,8 @@ private long configureForVirtualDisplay(
// Configures the view for Texture Layer Hybrid Composition mode, returning the associated
// texture ID.
@TargetApi(23)
private long configureForTextureLayerComposition(
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public long configureForTextureLayerComposition(
@NonNull PlatformView platformView,
@NonNull PlatformViewsChannel.PlatformViewCreationRequest request) {
// This mode attaches the view to the Android view hierarchy and record its drawing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import io.flutter.embedding.engine.systemchannels.AccessibilityChannel;
import io.flutter.embedding.engine.systemchannels.MouseCursorChannel;
import io.flutter.embedding.engine.systemchannels.PlatformViewsChannel;
import io.flutter.embedding.engine.systemchannels.PlatformViewsChannel.PlatformViewTouch;
import io.flutter.embedding.engine.systemchannels.SettingsChannel;
import io.flutter.embedding.engine.systemchannels.TextInputChannel;
import io.flutter.plugin.common.MethodCall;
Expand Down Expand Up @@ -79,6 +80,8 @@ public CountingPlatformView(Context context) {

@Override
public void dispose() {
// We have been removed from the view hierarhy before the call to dispose.
assertNull(view.getParent());
disposeCalls++;
}

Expand All @@ -98,6 +101,46 @@ public void onFlutterViewDetached() {
}
}

@Test
@Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class})
public void itRemovesPlatformViewBeforeDiposeIsCalled() {
PlatformViewsController platformViewsController = new PlatformViewsController();
FlutterJNI jni = new FlutterJNI();
attach(jni, platformViewsController);
// Get the platform view registry.
PlatformViewRegistry registry = platformViewsController.getRegistry();

// Register a factory for our platform view.
registry.registerViewFactory(
CountingPlatformView.VIEW_TYPE_ID,
new PlatformViewFactory(StandardMessageCodec.INSTANCE) {
@Override
public PlatformView create(Context context, int viewId, Object args) {
return new CountingPlatformView(context);
}
});

// Create the platform view.
int viewId = 0;
final PlatformViewsChannel.PlatformViewCreationRequest request =
new PlatformViewsChannel.PlatformViewCreationRequest(
viewId,
CountingPlatformView.VIEW_TYPE_ID,
0,
0,
128,
128,
View.LAYOUT_DIRECTION_LTR,
null);
PlatformView pView = platformViewsController.createPlatformView(request, true);
assertTrue(pView instanceof CountingPlatformView);
CountingPlatformView cpv = (CountingPlatformView) pView;
platformViewsController.configureForTextureLayerComposition(pView, request);
assertEquals(0, cpv.disposeCalls);
platformViewsController.disposePlatformView(viewId);
assertEquals(1, cpv.disposeCalls);
}

@Test
@Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class})
public void itNotifiesPlatformViewsOfEngineAttachmentAndDetachment() {
Expand All @@ -121,7 +164,7 @@ public PlatformView create(Context context, int viewId, Object args) {
int viewId = 0;
final PlatformViewsChannel.PlatformViewCreationRequest request =
new PlatformViewsChannel.PlatformViewCreationRequest(
viewId++,
viewId,
CountingPlatformView.VIEW_TYPE_ID,
0,
0,
Expand Down

0 comments on commit 69c76a4

Please sign in to comment.