Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Commit

Permalink
Don't call PlatformViewCreatedCallbacks after `AndroidViewControlle…
Browse files Browse the repository at this point in the history
…r` 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
  • Loading branch information
blaugold committed Feb 8, 2023
1 parent bfea22d commit ec289f1
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 10 deletions.
11 changes: 6 additions & 5 deletions packages/flutter/lib/src/services/platform_views.dart
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ abstract class AndroidViewController extends PlatformViewController {
Future<void> setLayoutDirection(TextDirection layoutDirection) async {
assert(
_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) {
Expand Down Expand Up @@ -938,12 +938,13 @@ abstract class AndroidViewController extends PlatformViewController {
/// disposed.
@override
Future<void> dispose() async {
if (_state == _AndroidViewState.creating || _state == _AndroidViewState.created) {
await _sendDisposeMessage();
}
_platformViewCreatedCallbacks.clear();
final _AndroidViewState state = _state;
_state = _AndroidViewState.disposed;
_platformViewCreatedCallbacks.clear();
PlatformViewsService._instance._focusCallbacks.remove(viewId);
if (state == _AndroidViewState.creating || state == _AndroidViewState.created) {
await _sendDisposeMessage();
}
}
}

Expand Down
22 changes: 17 additions & 5 deletions packages/flutter/lib/src/widgets/platform_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ class _AndroidViewState extends State<AndroidView> {
_layoutDirection = newLayoutDirection;

if (widget.viewType != oldWidget.viewType) {
_controller.dispose();
_controller.disposePostFrame();
_createNewAndroidView();
return;
}
Expand Down Expand Up @@ -890,7 +890,7 @@ class _PlatformViewLinkState extends State<PlatformViewLink> {
super.didUpdateWidget(oldWidget);

if (widget.viewType != oldWidget.viewType) {
_controller?.dispose();
_controller?.disposePostFrame();
// The _surface has to be recreated as its controller is disposed.
// Setting _surface to null will trigger its creation in build().
_surface = null;
Expand All @@ -911,9 +911,11 @@ class _PlatformViewLinkState extends State<PlatformViewLink> {
}

void _onPlatformViewCreated(int id) {
setState(() {
_platformViewCreated = true;
});
if (mounted) {
setState(() {
_platformViewCreated = true;
});
}
}

void _handleFrameworkFocusChanged(bool isFocused) {
Expand Down Expand Up @@ -1209,3 +1211,13 @@ class _PlatformViewPlaceHolder extends SingleChildRenderObjectWidget {
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();
});
}
}
13 changes: 13 additions & 0 deletions packages/flutter/test/services/platform_views_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,19 @@ void main() {
await controller2.create(size: const Size(100.0, 200.0));
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 {
Expand Down
26 changes: 26 additions & 0 deletions packages/flutter/test/widgets/platform_view_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2718,6 +2718,32 @@ void main() {
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 {
final GlobalKey key = GlobalKey();
final int currentViewId = platformViewsRegistry.getNextPlatformViewId();
Expand Down

0 comments on commit ec289f1

Please sign in to comment.