From 85101fd1f92d78daab6778c8297a212936ce7f9b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 1 Aug 2024 14:33:36 -0700 Subject: [PATCH 1/7] [android] enqueue platform views for deletion. --- .../platform/PlatformViewsController.java | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 9c78a3f030135..d2d2952755b09 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -118,6 +118,9 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega // Map of unique IDs to views that render overlay layers. private final SparseArray overlayLayerViews; + // Platform views to be disposed on the next frame. + private final ArrayList platformViewsToDispose; + // The platform view wrappers that are appended to FlutterView. // // These platform views use a TextureLayer in the framework. This is different than @@ -280,18 +283,9 @@ public void dispose(int viewId) { viewWrappers.remove(viewId); return; } - // The platform view is displayed using a PlatformViewLayer. - final FlutterMutatorView parentView = platformViewParent.get(viewId); - if (parentView != null) { - parentView.removeAllViews(); - parentView.unsetOnDescendantFocusChangeListener(); - - final ViewGroup mutatorViewParent = (ViewGroup) parentView.getParent(); - if (mutatorViewParent != null) { - mutatorViewParent.removeView(parentView); - } - platformViewParent.remove(viewId); - } + // The platform view is displayed using a PlatformViewLayer. Enqueue + // the platform view for deletion during the next frame. + platformViewsToDispose.add(viewId); } @Override @@ -1247,6 +1241,27 @@ public void onBeginFrame() { *

This member is not intended for public use, and is only visible for testing. */ public void onEndFrame() { + if (Integer viewId : platformViewsToDispose) { + // Any platform views that are queued for disposal but still in the composition + // tree must survive at least one more frame, but otherwise can be deleted. + if (currentFrameUsedPlatformViewIds.contains(viewId)) { + continue; + } + + // Remove unused platform view. + final FlutterMutatorView parentView = platformViewParent.get(viewId); + if (parentView != null) { + parentView.removeAllViews(); + parentView.unsetOnDescendantFocusChangeListener(); + + final ViewGroup mutatorViewParent = (ViewGroup) parentView.getParent(); + if (mutatorViewParent != null) { + mutatorViewParent.removeView(parentView); + } + platformViewParent.remove(viewId); + } + } + // If there are no platform views in the current frame, // then revert the image view surface and use the previous surface. // From 36fa6068f8d9ea4e619ec69352eb70a7bc716ac5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 1 Aug 2024 14:35:37 -0700 Subject: [PATCH 2/7] clear viewIds. --- .../io/flutter/plugin/platform/PlatformViewsController.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index d2d2952755b09..2e3af24c529c4 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -119,7 +119,7 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega private final SparseArray overlayLayerViews; // Platform views to be disposed on the next frame. - private final ArrayList platformViewsToDispose; + private ArrayList platformViewsToDispose; // The platform view wrappers that are appended to FlutterView. // @@ -738,6 +738,7 @@ public PlatformViewsController() { viewWrappers = new SparseArray<>(); platformViews = new SparseArray<>(); platformViewParent = new SparseArray<>(); + platformViewsToDispose = new ArrayList<>(); motionEventTracker = MotionEventTracker.getInstance(); } @@ -1241,10 +1242,12 @@ public void onBeginFrame() { *

This member is not intended for public use, and is only visible for testing. */ public void onEndFrame() { + ArrayList nextPlatformViewsToDispose if (Integer viewId : platformViewsToDispose) { // Any platform views that are queued for disposal but still in the composition // tree must survive at least one more frame, but otherwise can be deleted. if (currentFrameUsedPlatformViewIds.contains(viewId)) { + nextPlatformViewsToDispose.add(viewId); continue; } @@ -1261,6 +1264,7 @@ public void onEndFrame() { platformViewParent.remove(viewId); } } + platformViewsToDispose = nextPlatformViewsToDispose; // If there are no platform views in the current frame, // then revert the image view surface and use the previous surface. From 0a9b101a1c8985cdcd6b6b7ab484c27399f97905 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 1 Aug 2024 14:51:07 -0700 Subject: [PATCH 3/7] can write java. --- .../io/flutter/plugin/platform/PlatformViewsController.java | 4 ++-- .../flutter/plugin/platform/PlatformViewsControllerTest.java | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 2e3af24c529c4..268b2281d7564 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -1242,8 +1242,8 @@ public void onBeginFrame() { *

This member is not intended for public use, and is only visible for testing. */ public void onEndFrame() { - ArrayList nextPlatformViewsToDispose - if (Integer viewId : platformViewsToDispose) { + ArrayList nextPlatformViewsToDispose = new ArrayList<>(); + for (Integer viewId : platformViewsToDispose) { // Any platform views that are queued for disposal but still in the composition // tree must survive at least one more frame, but otherwise can be deleted. if (currentFrameUsedPlatformViewIds.contains(viewId)) { 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 af3a79e7adc22..b97e9ae0c2e77 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -908,6 +908,10 @@ public void disposeAndroidView_hybridComposition() { // Simulate dispose call from the framework. disposePlatformView(jni, platformViewsController, platformViewId); + assertNotNull(androidView.getParent()); + + // pump frame to force disposal. + platformViewsController.onEndFrame(); assertNull(androidView.getParent()); // Simulate create call from the framework. From 3b1a451a86e58ad6e827cd387ef2ab672c217089 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 1 Aug 2024 16:43:55 -0700 Subject: [PATCH 4/7] ++ --- .../platform/PlatformViewsController.java | 31 ++++++++++++------- .../platform/PlatformViewsControllerTest.java | 3 ++ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 268b2281d7564..43c4288b2064d 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -1066,6 +1066,10 @@ private void diposeAllViews() { // Dispose deletes the entry from platformViews and clears associated resources. channelHandler.dispose(viewId); } + for (Integer viewId : platformViewsToDispose) { + disposeHybridCompositionPlatformView(viewId); + } + platformViewsToDispose.clear(); } // Invoked when the Android system is requesting we reduce memory usage. @@ -1251,18 +1255,7 @@ public void onEndFrame() { continue; } - // Remove unused platform view. - final FlutterMutatorView parentView = platformViewParent.get(viewId); - if (parentView != null) { - parentView.removeAllViews(); - parentView.unsetOnDescendantFocusChangeListener(); - - final ViewGroup mutatorViewParent = (ViewGroup) parentView.getParent(); - if (mutatorViewParent != null) { - mutatorViewParent.removeView(parentView); - } - platformViewParent.remove(viewId); - } + disposeHybridCompositionPlatformView(viewId); } platformViewsToDispose = nextPlatformViewsToDispose; @@ -1407,4 +1400,18 @@ private void removeOverlaySurfaces() { public SparseArray getOverlayLayerViews() { return overlayLayerViews; } + + private void disposeHybridCompositionPlatformView(int viewId) { + final FlutterMutatorView parentView = platformViewParent.get(viewId); + if (parentView != null) { + parentView.removeAllViews(); + parentView.unsetOnDescendantFocusChangeListener(); + + final ViewGroup mutatorViewParent = (ViewGroup) parentView.getParent(); + if (mutatorViewParent != null) { + mutatorViewParent.removeView(parentView); + } + platformViewParent.remove(viewId); + } + } } 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 b97e9ae0c2e77..6a421b16f1ad3 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -1094,6 +1094,9 @@ public void onEndFrame_removesPlatformViewParent() { // Simulate dispose call from the framework. disposePlatformView(jni, platformViewsController, platformViewId); + platformViewsController.onBeginFrame(); + platformViewsController.onEndFrame(); + assertEquals(flutterView.getChildCount(), 1); } From 4dc09e28abb2b2ae17ddb28b3734d7887af46142 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 2 Aug 2024 09:17:06 -0700 Subject: [PATCH 5/7] ++ --- .../platform/PlatformViewsController.java | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 43c4288b2064d..7ee99877ad17b 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -242,6 +242,16 @@ public void dispose(int viewId) { Log.e(TAG, "Disposing unknown platform view with id: " + viewId); return; } + + // The platform view is displayed using a PlatformViewLayer. Enqueue + // the platform view for deletion during the next frame. + final FlutterMutatorView parentView = platformViewParent.get(viewId); + if (parentView != null) { + platformViewsToDispose.add(viewId); + return; + } + + if (platformView.getView() != null) { final View embeddedView = platformView.getView(); final ViewGroup pvParent = (ViewGroup) embeddedView.getParent(); @@ -283,9 +293,6 @@ public void dispose(int viewId) { viewWrappers.remove(viewId); return; } - // The platform view is displayed using a PlatformViewLayer. Enqueue - // the platform view for deletion during the next frame. - platformViewsToDispose.add(viewId); } @Override @@ -1402,6 +1409,26 @@ public SparseArray getOverlayLayerViews() { } private void disposeHybridCompositionPlatformView(int viewId) { + final PlatformView platformView = platformViews.get(viewId); + if (platformView != null) { + 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); + } + } + final FlutterMutatorView parentView = platformViewParent.get(viewId); if (parentView != null) { parentView.removeAllViews(); From edbfca9984765ce37980f727a9cc51fcfd422e36 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 2 Aug 2024 09:18:53 -0700 Subject: [PATCH 6/7] ++ --- .../io/flutter/plugin/platform/PlatformViewsController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 7ee99877ad17b..bb824555b277b 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -251,7 +251,6 @@ public void dispose(int viewId) { return; } - if (platformView.getView() != null) { final View embeddedView = platformView.getView(); final ViewGroup pvParent = (ViewGroup) embeddedView.getParent(); From a71acbed1d7fd64d1ef1e6d758058812418c76bb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 2 Aug 2024 12:10:47 -0700 Subject: [PATCH 7/7] ++ --- .../flutter/plugin/platform/PlatformViewsControllerTest.java | 3 +++ 1 file changed, 3 insertions(+) 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 6a421b16f1ad3..9608bbf676c21 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -951,6 +951,9 @@ public void disposeNullAndroidView() { // Simulate dispose call from the framework. disposePlatformView(jni, platformViewsController, platformViewId); + // pump frame to force disposal. + platformViewsController.onEndFrame(); + verify(platformView, times(1)).dispose(); }