From 8fb082ebad1037512622d1d2af796bb596ea75f0 Mon Sep 17 00:00:00 2001 From: Kristoffer Vinther Date: Thu, 28 Apr 2022 14:58:55 +0200 Subject: [PATCH 1/6] [url-launcher] Fixed call to onPlatformViewCreated after dispose which caused an error, since it calls setState(). --- .../url_launcher_web/example/lib/main.dart | 57 ++++++++++++++++--- .../url_launcher_web/example/pubspec.yaml | 5 ++ .../url_launcher_web/lib/src/link.dart | 42 +++++++++++--- 3 files changed, 87 insertions(+), 17 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/example/lib/main.dart b/packages/url_launcher/url_launcher_web/example/lib/main.dart index 341913a18490..44e1b188ba34 100644 --- a/packages/url_launcher/url_launcher_web/example/lib/main.dart +++ b/packages/url_launcher/url_launcher_web/example/lib/main.dart @@ -2,24 +2,63 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; +import 'package:url_launcher/link.dart'; void main() { runApp(MyApp()); } +class _HorizontalScrollBehavior extends MaterialScrollBehavior { + @override + Set get dragDevices => { + PointerDeviceKind.touch, + PointerDeviceKind.stylus, + PointerDeviceKind.invertedStylus, + PointerDeviceKind.mouse, + }; +} + /// App for testing -class MyApp extends StatefulWidget { +class MyApp extends StatelessWidget { @override - _MyAppState createState() => _MyAppState(); + Widget build(BuildContext context) => MaterialApp( + home: Scaffold( + body: ConstrainedBox( + constraints: const BoxConstraints(maxHeight: 300), + child: const _List(), + ), + ), + ); } -class _MyAppState extends State { +class _List extends StatelessWidget { + const _List({ + Key? key, + }) : super(key: key); + @override - Widget build(BuildContext context) { - return const Directionality( - textDirection: TextDirection.ltr, - child: Text('Testing... Look at the console output for results!'), - ); - } + Widget build(BuildContext context) => ScrollConfiguration( + behavior: _HorizontalScrollBehavior(), + child: ListView.separated( + scrollDirection: Axis.horizontal, + itemCount: 500, + separatorBuilder: (_, __) => const SizedBox(height: 24, width: 24), + itemBuilder: (_, int index) => Link( + uri: Uri.parse('https://example.com/$index'), + builder: (_, __) => GestureDetector( + onTap: () { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text('https://example.com/$index'))); + }, + child: Card( + child: Padding( + padding: const EdgeInsets.all(64), + child: Text('#$index', textAlign: TextAlign.center), + )), + ), + ), + ), + ); } diff --git a/packages/url_launcher/url_launcher_web/example/pubspec.yaml b/packages/url_launcher/url_launcher_web/example/pubspec.yaml index bface463cfe2..43086b5e1c26 100644 --- a/packages/url_launcher/url_launcher_web/example/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/example/pubspec.yaml @@ -8,6 +8,11 @@ environment: dependencies: flutter: sdk: flutter + url_launcher: + path: ../../url_launcher/ +dependency_overrides: + url_launcher_web: + path: ../ dev_dependencies: build_runner: ^2.1.1 diff --git a/packages/url_launcher/url_launcher_web/lib/src/link.dart b/packages/url_launcher/url_launcher_web/lib/src/link.dart index 4498e74ea9ce..fe068cb0a76a 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -114,13 +114,24 @@ class LinkViewController extends PlatformViewController { factory LinkViewController.fromParams( PlatformViewCreationParams params, BuildContext context, - ) { - final int viewId = params.id; - final LinkViewController controller = LinkViewController(viewId, context); - controller._initialize().then((_) { + ) => + LinkViewController(params.id, context).._asyncInit(params); + + Future _asyncInit(PlatformViewCreationParams params) async { + try { + await _initialize(); + if (context == null) { + return; + } params.onPlatformViewCreated(viewId); - }); - return controller; + } catch (exception, stack) { + FlutterError.reportError(FlutterErrorDetails( + exception: exception, + stack: stack, + library: 'url_launcher', + context: ErrorDescription('while initializing view $viewId'), + )); + } } static final Map _instances = @@ -160,7 +171,7 @@ class LinkViewController extends PlatformViewController { final int viewId; /// The context of the [Link] widget that created this controller. - final BuildContext context; + BuildContext? context; late html.Element _element; @@ -263,14 +274,29 @@ class LinkViewController extends PlatformViewController { } @override - Future dispose() async { + Future dispose() { + context = null; if (_isInitialized) { assert(_instances[viewId] == this); _instances.remove(viewId); + return _asyncDispose(); + } + return Future.value(); + } + + Future _asyncDispose() async { + try { if (_instances.isEmpty) { await _clickSubscription.cancel(); } await SystemChannels.platform_views.invokeMethod('dispose', viewId); + } catch (exception, stack) { + FlutterError.reportError(FlutterErrorDetails( + exception: exception, + stack: stack, + library: 'url_launcher', + context: ErrorDescription('while disposing view $viewId'), + )); } } } From 3b09ff570d72689edb6d7d754a1533b67ea6b2a3 Mon Sep 17 00:00:00 2001 From: Kristoffer Vinther Date: Sun, 15 May 2022 12:00:07 +0200 Subject: [PATCH 2/6] [url_launcher_web] Simplified example app. --- .../url_launcher_web/example/lib/main.dart | 47 ++++--------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/example/lib/main.dart b/packages/url_launcher/url_launcher_web/example/lib/main.dart index a10ec8f4434e..27b8f000e671 100644 --- a/packages/url_launcher/url_launcher_web/example/lib/main.dart +++ b/packages/url_launcher/url_launcher_web/example/lib/main.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:url_launcher/link.dart'; @@ -10,58 +9,28 @@ void main() { runApp(const MyApp()); } -class _HorizontalScrollBehavior extends MaterialScrollBehavior { - @override - Set get dragDevices => { - PointerDeviceKind.touch, - PointerDeviceKind.stylus, - PointerDeviceKind.invertedStylus, - PointerDeviceKind.mouse, - }; -} - /// App for testing class MyApp extends StatelessWidget { /// Default Constructor const MyApp({Key? key}) : super(key: key); @override - Widget build(BuildContext context) => MaterialApp( + Widget build(BuildContext context) => const MaterialApp( home: Scaffold( - body: ConstrainedBox( - constraints: const BoxConstraints(maxHeight: 300), - child: const _List(), - ), + body: _List(), ), ); } class _List extends StatelessWidget { - const _List({ - Key? key, - }) : super(key: key); + const _List({Key? key}) : super(key: key); @override - Widget build(BuildContext context) => ScrollConfiguration( - behavior: _HorizontalScrollBehavior(), - child: ListView.separated( - scrollDirection: Axis.horizontal, - itemCount: 500, - separatorBuilder: (_, __) => const SizedBox(height: 24, width: 24), - itemBuilder: (_, int index) => Link( - uri: Uri.parse('https://example.com/$index'), - builder: (_, __) => GestureDetector( - onTap: () { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar(content: Text('https://example.com/$index'))); - }, - child: Card( - child: Padding( - padding: const EdgeInsets.all(64), - child: Text('#$index', textAlign: TextAlign.center), - )), - ), - ), + Widget build(BuildContext context) => ListView.builder( + itemCount: 5000, + itemBuilder: (_, int index) => Link( + uri: Uri.parse('https://example.com/$index'), + builder: (_, __) => Text('#$index', textAlign: TextAlign.center), ), ); } From 0354b1d49f5f098914ec5d00c8169bef1caffaa1 Mon Sep 17 00:00:00 2001 From: Kristoffer Vinther Date: Sun, 15 May 2022 12:01:13 +0200 Subject: [PATCH 3/6] [url_launcher_web] Simplified LinkViewController state. --- .../url_launcher_web/lib/src/link.dart | 69 +++++++++---------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/lib/src/link.dart b/packages/url_launcher/url_launcher_web/lib/src/link.dart index 40a8dddfc372..b0adc00185ea 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -76,7 +76,7 @@ class WebLinkDelegateState extends State { child: PlatformViewLink( viewType: linkViewType, onCreatePlatformView: (PlatformViewCreationParams params) { - _controller = LinkViewController.fromParams(params, context); + _controller = LinkViewController.fromParams(params); return _controller ..setUri(widget.link.uri) ..setTarget(widget.link.target); @@ -100,7 +100,7 @@ class WebLinkDelegateState extends State { /// Controls link views. class LinkViewController extends PlatformViewController { /// Creates a [LinkViewController] instance with the unique [viewId]. - LinkViewController(this.viewId, this.context) { + LinkViewController(this.viewId) : _element = _makeElement(viewId) { if (_instances.isEmpty) { // This is the first controller being created, attach the global click // listener. @@ -113,14 +113,17 @@ class LinkViewController extends PlatformViewController { /// platform view [params]. factory LinkViewController.fromParams( PlatformViewCreationParams params, - BuildContext context, ) => - LinkViewController(params.id, context).._asyncInit(params); + LinkViewController(params.id).._asyncInitialize(params); - Future _asyncInit(PlatformViewCreationParams params) async { + Future _asyncInitialize(PlatformViewCreationParams params) async { try { - await _initialize(); - if (context == null) { + await SystemChannels.platform_views + .invokeMethod('create', { + 'id': viewId, + 'viewType': linkViewType, + }); + if (_isDisposed) { return; } params.onPlatformViewCreated(viewId); @@ -170,33 +173,9 @@ class LinkViewController extends PlatformViewController { @override final int viewId; - /// The context of the [Link] widget that created this controller. - BuildContext? context; + html.Element _element; - late html.Element _element; - - bool get _isInitialized => _element != null; - - Future _initialize() async { - _element = html.Element.tag('a'); - setProperty(_element, linkViewIdProperty, viewId); - _element.style - ..opacity = '0' - ..display = 'block' - ..width = '100%' - ..height = '100%' - ..cursor = 'unset'; - - // This is recommended on MDN: - // - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target - _element.setAttribute('rel', 'noreferrer noopener'); - - final Map args = { - 'id': viewId, - 'viewType': linkViewType, - }; - await SystemChannels.platform_views.invokeMethod('create', args); - } + bool _isDisposed = false; void _onDomClick(html.MouseEvent event) { final bool isHitTested = _hitTestedViewId == viewId; @@ -219,7 +198,7 @@ class LinkViewController extends PlatformViewController { // browser handle it. event.preventDefault(); final String routeName = _uri.toString(); - pushRouteNameToFramework(context, routeName); + pushRouteNameToFramework(null, routeName); } Uri? _uri; @@ -228,7 +207,6 @@ class LinkViewController extends PlatformViewController { /// /// When Uri is null, the `href` attribute of the link is removed. void setUri(Uri? uri) { - assert(_isInitialized); _uri = uri; if (uri == null) { _element.removeAttribute('href'); @@ -245,7 +223,6 @@ class LinkViewController extends PlatformViewController { /// Set the [LinkTarget] value for this link. void setTarget(LinkTarget target) { - assert(_isInitialized); _element.setAttribute('target', _getHtmlTarget(target)); } @@ -275,10 +252,10 @@ class LinkViewController extends PlatformViewController { @override Future dispose() { - context = null; - if (_isInitialized) { + if (!_isDisposed) { assert(_instances[viewId] == this); _instances.remove(viewId); + _isDisposed = true; return _asyncDispose(); } return Future.value(); @@ -301,6 +278,22 @@ class LinkViewController extends PlatformViewController { } } +html.Element _makeElement(int viewId) { + final html.Element element = html.Element.tag('a'); + setProperty(element, linkViewIdProperty, viewId); + element.style + ..opacity = '0' + ..display = 'block' + ..width = '100%' + ..height = '100%' + ..cursor = 'unset'; + + // This is recommended on MDN: + // - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target + element.setAttribute('rel', 'noreferrer noopener'); + return element; +} + /// Finds the view id of the DOM element targeted by the [event]. int? getViewIdFromTarget(html.Event event) { final html.Element? linkElement = getLinkElementFromTarget(event); From c49ce9b981d07f58871be959c0e434510aa9e155 Mon Sep 17 00:00:00 2001 From: Kristoffer Vinther Date: Sun, 15 May 2022 12:09:11 +0200 Subject: [PATCH 4/6] [url_launcher_web] Version bump. --- packages/url_launcher/url_launcher_web/CHANGELOG.md | 4 ++++ .../url_launcher/url_launcher_web/example/pubspec.yaml | 7 +------ packages/url_launcher/url_launcher_web/pubspec.yaml | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/CHANGELOG.md b/packages/url_launcher/url_launcher_web/CHANGELOG.md index 068650be6d53..06fa5e7ced7d 100644 --- a/packages/url_launcher/url_launcher_web/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_web/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.0.12 + +* Fixed call to `setState` after dispose. + ## 2.0.11 * Minor fixes for new analysis options. diff --git a/packages/url_launcher/url_launcher_web/example/pubspec.yaml b/packages/url_launcher/url_launcher_web/example/pubspec.yaml index 43086b5e1c26..06439d109864 100644 --- a/packages/url_launcher/url_launcher_web/example/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/example/pubspec.yaml @@ -9,10 +9,7 @@ dependencies: flutter: sdk: flutter url_launcher: - path: ../../url_launcher/ -dependency_overrides: - url_launcher_web: - path: ../ + path: ../../url_launcher dev_dependencies: build_runner: ^2.1.1 @@ -23,5 +20,3 @@ dev_dependencies: integration_test: sdk: flutter mockito: ^5.0.0 - url_launcher_web: - path: ../ diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index cef323035379..d0e0fa905d57 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/pubspec.yaml @@ -2,7 +2,7 @@ name: url_launcher_web description: Web platform implementation of url_launcher repository: https://github.com/flutter/plugins/tree/main/packages/url_launcher/url_launcher_web issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22 -version: 2.0.11 +version: 2.0.12 environment: sdk: ">=2.12.0 <3.0.0" From 00f0f2a223124d67c7bb8d66aa8e5c717eda6661 Mon Sep 17 00:00:00 2001 From: Kristoffer Vinther Date: Mon, 16 May 2022 18:14:16 +0200 Subject: [PATCH 5/6] [url_launcher_web] Revert changes to example app. --- .../url_launcher_web/example/lib/main.dart | 26 +++++++------------ .../url_launcher_web/example/pubspec.yaml | 4 +-- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/example/lib/main.dart b/packages/url_launcher/url_launcher_web/example/lib/main.dart index 27b8f000e671..87422953de6a 100644 --- a/packages/url_launcher/url_launcher_web/example/lib/main.dart +++ b/packages/url_launcher/url_launcher_web/example/lib/main.dart @@ -3,34 +3,26 @@ // found in the LICENSE file. import 'package:flutter/material.dart'; -import 'package:url_launcher/link.dart'; void main() { runApp(const MyApp()); } /// App for testing -class MyApp extends StatelessWidget { +class MyApp extends StatefulWidget { /// Default Constructor const MyApp({Key? key}) : super(key: key); @override - Widget build(BuildContext context) => const MaterialApp( - home: Scaffold( - body: _List(), - ), - ); + State createState() => _MyAppState(); } -class _List extends StatelessWidget { - const _List({Key? key}) : super(key: key); - +class _MyAppState extends State { @override - Widget build(BuildContext context) => ListView.builder( - itemCount: 5000, - itemBuilder: (_, int index) => Link( - uri: Uri.parse('https://example.com/$index'), - builder: (_, __) => Text('#$index', textAlign: TextAlign.center), - ), - ); + Widget build(BuildContext context) { + return const Directionality( + textDirection: TextDirection.ltr, + child: Text('Testing... Look at the console output for results!'), + ); + } } diff --git a/packages/url_launcher/url_launcher_web/example/pubspec.yaml b/packages/url_launcher/url_launcher_web/example/pubspec.yaml index 06439d109864..bface463cfe2 100644 --- a/packages/url_launcher/url_launcher_web/example/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/example/pubspec.yaml @@ -8,8 +8,6 @@ environment: dependencies: flutter: sdk: flutter - url_launcher: - path: ../../url_launcher dev_dependencies: build_runner: ^2.1.1 @@ -20,3 +18,5 @@ dev_dependencies: integration_test: sdk: flutter mockito: ^5.0.0 + url_launcher_web: + path: ../ From 7b5e35a812dbaa134b23b80b51fb41aa760e5f8f Mon Sep 17 00:00:00 2001 From: Kristoffer Vinther Date: Mon, 16 May 2022 18:14:19 +0200 Subject: [PATCH 6/6] [url_launcher_web] Added stress test of creation and disposing of Link widgets. --- .../integration_test/link_widget_test.dart | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart b/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart index 3b75e0556686..6b19861c5ce5 100644 --- a/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart +++ b/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart @@ -123,6 +123,36 @@ void main() { final html.Element anchor = _findSingleAnchor(); expect(anchor.hasAttribute('href'), false); }); + + testWidgets('can be created and disposed', (WidgetTester tester) async { + final Uri uri = Uri.parse('http://foobar'); + const int itemCount = 500; + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: MediaQuery( + data: const MediaQueryData(), + child: ListView.builder( + itemCount: itemCount, + itemBuilder: (_, int index) => WebLinkDelegate(TestLinkInfo( + uri: uri, + target: LinkTarget.defaultTarget, + builder: (BuildContext context, FollowLink? followLink) => + Text('#$index', textAlign: TextAlign.center), + )), + ), + ), + ), + ); + + await tester.pumpAndSettle(); + + await tester.scrollUntilVisible( + find.text('#${itemCount - 1}'), + 2500, + maxScrolls: 1000, + ); + }); }); }