From 5c7a90b77569af89415c1048197183d5474730c5 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 2 Feb 2022 22:22:59 -0800 Subject: [PATCH] Don't remove views while the rasterizer is torn down --- .../platform/PlatformViewsController.java | 22 +++++-- .../platform/PlatformViewsControllerTest.java | 63 ++++++++++++++----- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index bc69abf0796ed..29ace9e502c2d 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -508,6 +508,7 @@ public void attachToView(@NonNull FlutterView flutterView) { */ public void detachFromView() { destroyOverlaySurfaces(); + removeOverlaySurfaces(); this.flutterView = null; flutterViewConvertedToImageView = false; @@ -970,17 +971,26 @@ public FlutterOverlaySurface createOverlaySurface() { * Destroys the overlay surfaces and removes them from the view hierarchy. * *

This method is used only internally by {@code FlutterJNI}. - * - *

This member is not intended for public use, and is only visible for testing. */ public void destroyOverlaySurfaces() { for (int i = 0; i < overlayLayerViews.size(); i++) { - FlutterImageView overlayView = overlayLayerViews.valueAt(i); + final FlutterImageView overlayView = overlayLayerViews.valueAt(i); overlayView.detachFromRenderer(); overlayView.closeImageReader(); - if (flutterView != null) { - flutterView.removeView(overlayView); - } + // Don't remove overlayView from the view hierarchy since this method can + // be called while the Android framework is iterating over the array of views. + // See ViewGroup#dispatchDetachedFromWindow(), and + // https://github.com/flutter/flutter/issues/97679. + } + } + + private void removeOverlaySurfaces() { + if (flutterView == null) { + Log.e(TAG, "removeOverlaySurfaces called while flutter view is null"); + return; + } + for (int i = 0; i < overlayLayerViews.size(); i++) { + flutterView.removeView(overlayLayerViews.valueAt(i)); } overlayLayerViews.clear(); } diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index 17d1caefae548..c62d2e9050b78 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -657,17 +657,13 @@ public void detach__destroysOverlaySurfaces() { platformViewsController.detach(); - assertThrows( - IllegalStateException.class, - () -> { - platformViewsController.onDisplayOverlaySurface( - overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); - }); + verify(overlayImageView, times(1)).closeImageReader(); + verify(overlayImageView, times(1)).detachFromRenderer(); } @Test @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) - public void detachFromView__removesOverlaySurfaces() { + public void detachFromView__removesAndDestroysOverlayViews() { final PlatformViewsController platformViewsController = new PlatformViewsController(); final int platformViewId = 0; @@ -684,6 +680,9 @@ public void detachFromView__removesOverlaySurfaces() { jni.attachToNative(); attach(jni, platformViewsController); + final FlutterView flutterView = mock(FlutterView.class); + platformViewsController.attachToView(flutterView); + final FlutterImageView overlayImageView = mock(FlutterImageView.class); when(overlayImageView.acquireLatestImage()).thenReturn(true); @@ -695,17 +694,14 @@ public void detachFromView__removesOverlaySurfaces() { platformViewsController.detachFromView(); - assertThrows( - IllegalStateException.class, - () -> { - platformViewsController.onDisplayOverlaySurface( - overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); - }); + verify(overlayImageView, times(1)).closeImageReader(); + verify(overlayImageView, times(1)).detachFromRenderer(); + verify(flutterView, times(1)).removeView(overlayImageView); } @Test @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) - public void destroyOverlaySurfaces__doesNotThrowIfControllerIsDetached() { + public void destroyOverlaySurfaces__doesNotThrowIfFlutterViewIsDetached() { final PlatformViewsController platformViewsController = new PlatformViewsController(); final int platformViewId = 0; @@ -722,6 +718,9 @@ public void destroyOverlaySurfaces__doesNotThrowIfControllerIsDetached() { jni.attachToNative(); attach(jni, platformViewsController); + final FlutterView flutterView = mock(FlutterView.class); + platformViewsController.attachToView(flutterView); + final FlutterImageView overlayImageView = mock(FlutterImageView.class); when(overlayImageView.acquireLatestImage()).thenReturn(true); @@ -735,6 +734,42 @@ public void destroyOverlaySurfaces__doesNotThrowIfControllerIsDetached() { platformViewsController.destroyOverlaySurfaces(); verify(overlayImageView, times(1)).closeImageReader(); + verify(overlayImageView, times(1)).detachFromRenderer(); + } + + @Test + @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + public void destroyOverlaySurfaces__doesNotRemoveOverlayView() { + final PlatformViewsController platformViewsController = new PlatformViewsController(); + + final int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + final PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + final PlatformView platformView = mock(PlatformView.class); + when(platformView.getView()).thenReturn(mock(View.class)); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + final FlutterJNI jni = new FlutterJNI(); + jni.attachToNative(); + attach(jni, platformViewsController); + + final FlutterView flutterView = mock(FlutterView.class); + platformViewsController.attachToView(flutterView); + + final FlutterImageView overlayImageView = mock(FlutterImageView.class); + when(overlayImageView.acquireLatestImage()).thenReturn(true); + + final FlutterOverlaySurface overlaySurface = + platformViewsController.createOverlaySurface(overlayImageView); + + platformViewsController.onDisplayOverlaySurface( + overlaySurface.getId(), /* x=*/ 0, /* y=*/ 0, /* width=*/ 10, /* height=*/ 10); + + platformViewsController.destroyOverlaySurfaces(); + verify(flutterView, never()).removeView(overlayImageView); } @Test