Unverified Commit ec289f1e authored by Gabriel Terwesten's avatar Gabriel Terwesten Committed by GitHub

Don't call `PlatformViewCreatedCallback`s after `AndroidViewController` is disposed (#116854)

* Don't call `PlatformViewCreatedCallback`s after `AndroidViewController` is disposed

Before this change it was possible that, if a `AndroidViewController` was disposed before we got the notification that the platform view was created, `PlatformViewCreatedCallback`s where called even after calling `AndroidViewController.dispose`.

Also makes `_PlatformViewLinkState._onPlatformViewCreated` more carful to only call `setState` when mounted.

Closes #84628
Closes #96384

* Allow all widgets to remove listeners from controller

* Remove assert

* Add expectations to test
parent bfea22db
...@@ -863,7 +863,7 @@ abstract class AndroidViewController extends PlatformViewController { ...@@ -863,7 +863,7 @@ abstract class AndroidViewController extends PlatformViewController {
Future<void> setLayoutDirection(TextDirection layoutDirection) async { Future<void> setLayoutDirection(TextDirection layoutDirection) async {
assert( assert(
_state != _AndroidViewState.disposed, _state != _AndroidViewState.disposed,
'trying to set a layout direction for a disposed UIView. View id: $viewId', 'trying to set a layout direction for a disposed Android view. View id: $viewId',
); );
if (layoutDirection == _layoutDirection) { if (layoutDirection == _layoutDirection) {
...@@ -938,12 +938,13 @@ abstract class AndroidViewController extends PlatformViewController { ...@@ -938,12 +938,13 @@ abstract class AndroidViewController extends PlatformViewController {
/// disposed. /// disposed.
@override @override
Future<void> dispose() async { Future<void> dispose() async {
if (_state == _AndroidViewState.creating || _state == _AndroidViewState.created) { final _AndroidViewState state = _state;
await _sendDisposeMessage();
}
_platformViewCreatedCallbacks.clear();
_state = _AndroidViewState.disposed; _state = _AndroidViewState.disposed;
_platformViewCreatedCallbacks.clear();
PlatformViewsService._instance._focusCallbacks.remove(viewId); PlatformViewsService._instance._focusCallbacks.remove(viewId);
if (state == _AndroidViewState.creating || state == _AndroidViewState.created) {
await _sendDisposeMessage();
}
} }
} }
......
...@@ -485,7 +485,7 @@ class _AndroidViewState extends State<AndroidView> { ...@@ -485,7 +485,7 @@ class _AndroidViewState extends State<AndroidView> {
_layoutDirection = newLayoutDirection; _layoutDirection = newLayoutDirection;
if (widget.viewType != oldWidget.viewType) { if (widget.viewType != oldWidget.viewType) {
_controller.dispose(); _controller.disposePostFrame();
_createNewAndroidView(); _createNewAndroidView();
return; return;
} }
...@@ -890,7 +890,7 @@ class _PlatformViewLinkState extends State<PlatformViewLink> { ...@@ -890,7 +890,7 @@ class _PlatformViewLinkState extends State<PlatformViewLink> {
super.didUpdateWidget(oldWidget); super.didUpdateWidget(oldWidget);
if (widget.viewType != oldWidget.viewType) { if (widget.viewType != oldWidget.viewType) {
_controller?.dispose(); _controller?.disposePostFrame();
// The _surface has to be recreated as its controller is disposed. // The _surface has to be recreated as its controller is disposed.
// Setting _surface to null will trigger its creation in build(). // Setting _surface to null will trigger its creation in build().
_surface = null; _surface = null;
...@@ -911,9 +911,11 @@ class _PlatformViewLinkState extends State<PlatformViewLink> { ...@@ -911,9 +911,11 @@ class _PlatformViewLinkState extends State<PlatformViewLink> {
} }
void _onPlatformViewCreated(int id) { void _onPlatformViewCreated(int id) {
setState(() { if (mounted) {
_platformViewCreated = true; setState(() {
}); _platformViewCreated = true;
});
}
} }
void _handleFrameworkFocusChanged(bool isFocused) { void _handleFrameworkFocusChanged(bool isFocused) {
...@@ -1209,3 +1211,13 @@ class _PlatformViewPlaceHolder extends SingleChildRenderObjectWidget { ...@@ -1209,3 +1211,13 @@ class _PlatformViewPlaceHolder extends SingleChildRenderObjectWidget {
renderObject.onLayout = onLayout; renderObject.onLayout = onLayout;
} }
} }
extension on PlatformViewController {
/// Disposes the controller in a post-frame callback, to allow other widgets to
/// remove their listeners before the controller is disposed.
void disposePostFrame() {
SchedulerBinding.instance.addPostFrameCallback((_) {
dispose();
});
}
}
...@@ -268,6 +268,19 @@ void main() { ...@@ -268,6 +268,19 @@ void main() {
await controller2.create(size: const Size(100.0, 200.0)); await controller2.create(size: const Size(100.0, 200.0));
expect(createdViews, orderedEquals(<int>[0, 5])); expect(createdViews, orderedEquals(<int>[0, 5]));
final AndroidViewController controller3 = PlatformViewsService.initAndroidView(
id: 10,
viewType: 'webview',
layoutDirection: TextDirection.ltr,
)..addOnPlatformViewCreatedListener(callback);
expect(createdViews, orderedEquals(<int>[0, 5]));
await Future.wait(<Future<void>>[
controller3.create(size: const Size(100.0, 200.0)),
controller3.dispose(),
]);
expect(createdViews, orderedEquals(<int>[0, 5]));
}); });
test("change Android view's directionality before creation", () async { test("change Android view's directionality before creation", () async {
......
...@@ -2718,6 +2718,32 @@ void main() { ...@@ -2718,6 +2718,32 @@ void main() {
expect(disposedController.disposed, true); expect(disposedController.disposed, true);
}); });
testWidgets('PlatformViewLink handles onPlatformViewCreated when disposed', (WidgetTester tester) async {
late PlatformViewCreationParams creationParams;
late FakePlatformViewController controller;
final PlatformViewLink platformViewLink = PlatformViewLink(
viewType: 'webview',
onCreatePlatformView: (PlatformViewCreationParams params) {
creationParams = params;
return controller = FakePlatformViewController(params.id);
},
surfaceFactory: (BuildContext context, PlatformViewController controller) {
return PlatformViewSurface(
gestureRecognizers: const <Factory<OneSequenceGestureRecognizer>>{},
controller: controller,
hitTestBehavior: PlatformViewHitTestBehavior.opaque,
);
},
);
await tester.pumpWidget(platformViewLink);
await tester.pumpWidget(Container());
expect(controller.disposed, true);
expect(() => creationParams.onPlatformViewCreated(creationParams.id), returnsNormally);
});
testWidgets('PlatformViewLink widget survives widget tree change', (WidgetTester tester) async { testWidgets('PlatformViewLink widget survives widget tree change', (WidgetTester tester) async {
final GlobalKey key = GlobalKey(); final GlobalKey key = GlobalKey();
final int currentViewId = platformViewsRegistry.getNextPlatformViewId(); final int currentViewId = platformViewsRegistry.getNextPlatformViewId();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment