From 63478a1a43ab1fa572d98b8d9d933c86a5aa5b4a Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Mon, 27 Jun 2022 15:05:16 -0700 Subject: [PATCH 01/17] prevent leaks --- .../lib/src/foundation/foundation.dart | 11 +++- .../lib/src/web_kit/web_kit.dart | 16 ++++++ .../lib/src/web_kit_webview_widget.dart | 50 +++++++++++++------ 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart index f537a4454c4f..ea8a73d3046d 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart @@ -267,7 +267,16 @@ class NSObject with Copyable { final NSObjectHostApiImpl _api; - /// Informs the observing object when the value at the specified key path has changed. + /// Informs the observing object when the value at the specified key path has + /// changed. + /// + /// {@template webview_flutter_wkwebview.foundation.callbacks} + /// It is strongly recommended that this Function doesn't contain a strong + /// reference to the encapsulating class instance. If done, the associated + /// Objective-C object would fail to be automatically garbage collected. + /// Consider using `WeakReference` when using objects not passed as a + /// parameter. + /// {@endtemplate} final void Function( String keyPath, NSObject object, diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart index ffc6eb8c23bf..a671064ddc0f 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart @@ -442,6 +442,8 @@ class WKScriptMessageHandler extends NSObject { /// Use this method to respond to a message sent from the webpage’s /// JavaScript code. Use the [message] parameter to get the message contents and /// to determine the originating web view. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function( WKUserContentController userContentController, WKScriptMessage message, @@ -733,6 +735,8 @@ class WKUIDelegate extends NSObject { final WKUIDelegateHostApiImpl _uiDelegateApi; /// Indicates a new [WKWebView] was requested to be created with [configuration]. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function( WKWebView webView, WKWebViewConfiguration configuration, @@ -803,26 +807,38 @@ class WKNavigationDelegate extends NSObject { final WKNavigationDelegateHostApiImpl _navigationDelegateApi; /// Called when navigation is complete. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function(WKWebView webView, String? url)? didFinishNavigation; /// Called when navigation from the main frame has started. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function(WKWebView webView, String? url)? didStartProvisionalNavigation; /// Called when permission is needed to navigate to new content. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final Future Function( WKWebView webView, WKNavigationAction navigationAction, )? decidePolicyForNavigationAction; /// Called when an error occurred during navigation. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function(WKWebView webView, NSError error)? didFailNavigation; /// Called when an error occurred during the early navigation process. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function(WKWebView webView, NSError error)? didFailProvisionalNavigation; /// Called when the web view’s content process was terminated. + /// + /// {@macro webview_flutter_wkwebview.foundation.callbacks} final void Function(WKWebView webView)? webViewWebContentProcessDidTerminate; @override diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index ed1912ee7898..774036e0c64f 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -130,20 +130,24 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { late final WKNavigationDelegate navigationDelegate = webViewProxy.createNavigationDelegate( didFinishNavigation: (WKWebView webView, String? url) { - callbacksHandler.onPageFinished(url ?? ''); + _weakSelf.target?.callbacksHandler.onPageFinished(url ?? ''); }, didStartProvisionalNavigation: (WKWebView webView, String? url) { - callbacksHandler.onPageStarted(url ?? ''); + _weakSelf.target?.callbacksHandler.onPageStarted(url ?? ''); }, decidePolicyForNavigationAction: ( WKWebView webView, WKNavigationAction action, ) async { - if (!_hasNavigationDelegate) { + if (_weakSelf.target == null) { + return WKNavigationActionPolicy.allow; + } + if (!_weakSelf.target!._hasNavigationDelegate) { return WKNavigationActionPolicy.allow; } - final bool allow = await callbacksHandler.onNavigationRequest( + final bool allow = + await _weakSelf.target!.callbacksHandler.onNavigationRequest( url: action.request.url, isForMainFrame: action.targetFrame.isMainFrame, ); @@ -153,13 +157,15 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { : WKNavigationActionPolicy.cancel; }, didFailNavigation: (WKWebView webView, NSError error) { - callbacksHandler.onWebResourceError(_toWebResourceError(error)); + _weakSelf.target?.callbacksHandler + .onWebResourceError(_toWebResourceError(error)); }, didFailProvisionalNavigation: (WKWebView webView, NSError error) { - callbacksHandler.onWebResourceError(_toWebResourceError(error)); + _weakSelf.target?.callbacksHandler + .onWebResourceError(_toWebResourceError(error)); }, webViewWebContentProcessDidTerminate: (WKWebView webView) { - callbacksHandler.onWebResourceError(WebResourceError( + _weakSelf.target?.callbacksHandler.onWebResourceError(WebResourceError( errorCode: WKErrorCode.webContentProcessTerminated, // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. domain: 'WKErrorDomain', @@ -169,6 +175,10 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { }, ); + // Callback methods require a weak reference to the instance they belong to. + late final WeakReference _weakSelf = + WeakReference(this); + Future _setCreationParams( CreationParams params, { required WKWebViewConfiguration configuration, @@ -185,7 +195,7 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { Map change, ) { final double progress = change[NSKeyValueChangeKey.newValue]! as double; - callbacksHandler.onProgress((progress * 100).round()); + _weakSelf.target?.callbacksHandler.onProgress((progress * 100).round()); }); webView.setUIDelegate(uiDelegate); @@ -408,21 +418,29 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { await Future.wait( javascriptChannelNames.where( (String channelName) { - return !_scriptMessageHandlers.containsKey(channelName); + if (_weakSelf.target == null) { + return false; + } + return !_weakSelf.target!._scriptMessageHandlers + .containsKey(channelName); }, ).map>( - (String channelName) { - final WKScriptMessageHandler handler = - webViewProxy.createScriptMessageHandler(didReceiveScriptMessage: ( + (String channelName) async { + if (_weakSelf.target == null) { + return; + } + final WKScriptMessageHandler handler = _weakSelf.target!.webViewProxy + .createScriptMessageHandler(didReceiveScriptMessage: ( WKUserContentController userContentController, WKScriptMessage message, ) { - javascriptChannelRegistry.onJavascriptChannelMessage( + _weakSelf.target!.javascriptChannelRegistry + .onJavascriptChannelMessage( message.name, message.body!.toString(), ); }); - _scriptMessageHandlers[channelName] = handler; + _weakSelf.target!._scriptMessageHandlers[channelName] = handler; final String wrapperSource = 'window.$channelName = webkit.messageHandlers.$channelName;'; @@ -431,9 +449,9 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { WKUserScriptInjectionTime.atDocumentStart, isMainFrameOnly: false, ); - webView.configuration.userContentController + _weakSelf.target!.webView.configuration.userContentController .addUserScript(wrapperScript); - return webView.configuration.userContentController + await _weakSelf.target!.webView.configuration.userContentController .addScriptMessageHandler( handler, channelName, From 1c8bd6ccb5b54ed4747a3d8899e03b6576daaabb Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Tue, 28 Jun 2022 14:55:20 -0700 Subject: [PATCH 02/17] change doc --- .../lib/src/foundation/foundation.dart | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart index ea8a73d3046d..ef08bed31f94 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart @@ -271,11 +271,12 @@ class NSObject with Copyable { /// changed. /// /// {@template webview_flutter_wkwebview.foundation.callbacks} - /// It is strongly recommended that this Function doesn't contain a strong - /// reference to the encapsulating class instance. If done, the associated - /// Objective-C object would fail to be automatically garbage collected. - /// Consider using `WeakReference` when using objects not passed as a - /// parameter. + /// For the associated Objective-C object to be automatically garbage + /// collected, it is required that this Function doesn't contain a strong + /// reference to the encapsulating class instance. Consider using + /// `WeakReference` when referencing an object not received as a parameter. + /// Otherwise, use [NSObject.dispose] to release the associated Objective-C + /// object manually. /// {@endtemplate} final void Function( String keyPath, From a41be67a7a374cbed2a62bd30a3c814acb9ad4de Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Tue, 28 Jun 2022 19:34:50 -0700 Subject: [PATCH 03/17] some experiments --- .../lib/src/common/weak_self_provider.dart | 7 + .../lib/src/web_kit_webview_widget.dart | 203 +++++++++--------- .../webview_flutter_wkwebview/pubspec.yaml | 1 + .../test/src/web_kit_webview_widget_test.dart | 90 +++++++- 4 files changed, 203 insertions(+), 98 deletions(-) create mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart new file mode 100644 index 000000000000..73e1468825a5 --- /dev/null +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart @@ -0,0 +1,7 @@ +/// +mixin WeakSelfProvider { + /// + S withWeakSelf(S Function(WeakReference weakSelf) onCreateWeakSelf) { + return onCreateWeakSelf(WeakReference(this as T)); + } +} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index 774036e0c64f..cda2d23ef95b 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -11,6 +11,7 @@ import 'package:flutter/services.dart'; import 'package:path/path.dart' as path; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; +import 'common/weak_self_provider.dart'; import 'foundation/foundation.dart'; import 'web_kit/web_kit.dart'; @@ -76,7 +77,8 @@ class _WebKitWebViewWidgetState extends State { } /// An implementation of [WebViewPlatformController] with the WebKit api. -class WebKitWebViewPlatformController extends WebViewPlatformController { +class WebKitWebViewPlatformController extends WebViewPlatformController + with WeakSelfProvider { /// Construct a [WebKitWebViewPlatformController]. WebKitWebViewPlatformController({ required CreationParams creationParams, @@ -127,58 +129,57 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { /// Methods for handling navigation changes and tracking navigation requests. @visibleForTesting - late final WKNavigationDelegate navigationDelegate = - webViewProxy.createNavigationDelegate( - didFinishNavigation: (WKWebView webView, String? url) { - _weakSelf.target?.callbacksHandler.onPageFinished(url ?? ''); - }, - didStartProvisionalNavigation: (WKWebView webView, String? url) { - _weakSelf.target?.callbacksHandler.onPageStarted(url ?? ''); - }, - decidePolicyForNavigationAction: ( - WKWebView webView, - WKNavigationAction action, - ) async { - if (_weakSelf.target == null) { - return WKNavigationActionPolicy.allow; - } - if (!_weakSelf.target!._hasNavigationDelegate) { - return WKNavigationActionPolicy.allow; - } + late final WKNavigationDelegate navigationDelegate = withWeakSelf( + (WeakReference weakSelf) { + return webViewProxy.createNavigationDelegate( + didFinishNavigation: (WKWebView webView, String? url) { + weakSelf.target?.callbacksHandler.onPageFinished(url ?? ''); + }, + didStartProvisionalNavigation: (WKWebView webView, String? url) { + weakSelf.target?.callbacksHandler.onPageStarted(url ?? ''); + }, + decidePolicyForNavigationAction: ( + WKWebView webView, + WKNavigationAction action, + ) async { + if (weakSelf.target == null) { + return WKNavigationActionPolicy.allow; + } + if (!weakSelf.target!._hasNavigationDelegate) { + return WKNavigationActionPolicy.allow; + } - final bool allow = - await _weakSelf.target!.callbacksHandler.onNavigationRequest( - url: action.request.url, - isForMainFrame: action.targetFrame.isMainFrame, - ); + final bool allow = + await weakSelf.target!.callbacksHandler.onNavigationRequest( + url: action.request.url, + isForMainFrame: action.targetFrame.isMainFrame, + ); - return allow - ? WKNavigationActionPolicy.allow - : WKNavigationActionPolicy.cancel; - }, - didFailNavigation: (WKWebView webView, NSError error) { - _weakSelf.target?.callbacksHandler - .onWebResourceError(_toWebResourceError(error)); - }, - didFailProvisionalNavigation: (WKWebView webView, NSError error) { - _weakSelf.target?.callbacksHandler - .onWebResourceError(_toWebResourceError(error)); - }, - webViewWebContentProcessDidTerminate: (WKWebView webView) { - _weakSelf.target?.callbacksHandler.onWebResourceError(WebResourceError( - errorCode: WKErrorCode.webContentProcessTerminated, - // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. - domain: 'WKErrorDomain', - description: '', - errorType: WebResourceErrorType.webContentProcessTerminated, - )); + return allow + ? WKNavigationActionPolicy.allow + : WKNavigationActionPolicy.cancel; + }, + didFailNavigation: (WKWebView webView, NSError error) { + weakSelf.target?.callbacksHandler + .onWebResourceError(_toWebResourceError(error)); + }, + didFailProvisionalNavigation: (WKWebView webView, NSError error) { + weakSelf.target?.callbacksHandler + .onWebResourceError(_toWebResourceError(error)); + }, + webViewWebContentProcessDidTerminate: (WKWebView webView) { + weakSelf.target?.callbacksHandler.onWebResourceError(WebResourceError( + errorCode: WKErrorCode.webContentProcessTerminated, + // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. + domain: 'WKErrorDomain', + description: '', + errorType: WebResourceErrorType.webContentProcessTerminated, + )); + }, + ); }, ); - // Callback methods require a weak reference to the instance they belong to. - late final WeakReference _weakSelf = - WeakReference(this); - Future _setCreationParams( CreationParams params, { required WKWebViewConfiguration configuration, @@ -189,13 +190,17 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { autoMediaPlaybackPolicy: params.autoMediaPlaybackPolicy, ); - webView = webViewProxy.createWebView(configuration, observeValue: ( - String keyPath, - NSObject object, - Map change, + webView = withWeakSelf(( + WeakReference weakSelf, ) { - final double progress = change[NSKeyValueChangeKey.newValue]! as double; - _weakSelf.target?.callbacksHandler.onProgress((progress * 100).round()); + return webViewProxy.createWebView(configuration, observeValue: ( + String keyPath, + NSObject object, + Map change, + ) { + final double progress = change[NSKeyValueChangeKey.newValue]! as double; + weakSelf.target?.callbacksHandler.onProgress((progress * 100).round()); + }); }); webView.setUIDelegate(uiDelegate); @@ -415,50 +420,54 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { @override Future addJavascriptChannels(Set javascriptChannelNames) async { - await Future.wait( - javascriptChannelNames.where( - (String channelName) { - if (_weakSelf.target == null) { - return false; - } - return !_weakSelf.target!._scriptMessageHandlers - .containsKey(channelName); - }, - ).map>( - (String channelName) async { - if (_weakSelf.target == null) { - return; - } - final WKScriptMessageHandler handler = _weakSelf.target!.webViewProxy - .createScriptMessageHandler(didReceiveScriptMessage: ( - WKUserContentController userContentController, - WKScriptMessage message, - ) { - _weakSelf.target!.javascriptChannelRegistry - .onJavascriptChannelMessage( - message.name, - message.body!.toString(), + await withWeakSelf(( + WeakReference weakSelf, + ) { + return Future.wait( + javascriptChannelNames.where( + (String channelName) { + if (weakSelf.target == null) { + return false; + } + return !weakSelf.target!._scriptMessageHandlers + .containsKey(channelName); + }, + ).map>( + (String channelName) async { + if (weakSelf.target == null) { + return; + } + final WKScriptMessageHandler handler = weakSelf.target!.webViewProxy + .createScriptMessageHandler(didReceiveScriptMessage: ( + WKUserContentController userContentController, + WKScriptMessage message, + ) { + weakSelf.target!.javascriptChannelRegistry + .onJavascriptChannelMessage( + message.name, + message.body!.toString(), + ); + }); + weakSelf.target!._scriptMessageHandlers[channelName] = handler; + + final String wrapperSource = + 'window.$channelName = webkit.messageHandlers.$channelName;'; + final WKUserScript wrapperScript = WKUserScript( + wrapperSource, + WKUserScriptInjectionTime.atDocumentStart, + isMainFrameOnly: false, ); - }); - _weakSelf.target!._scriptMessageHandlers[channelName] = handler; - - final String wrapperSource = - 'window.$channelName = webkit.messageHandlers.$channelName;'; - final WKUserScript wrapperScript = WKUserScript( - wrapperSource, - WKUserScriptInjectionTime.atDocumentStart, - isMainFrameOnly: false, - ); - _weakSelf.target!.webView.configuration.userContentController - .addUserScript(wrapperScript); - await _weakSelf.target!.webView.configuration.userContentController - .addScriptMessageHandler( - handler, - channelName, - ); - }, - ), - ); + weakSelf.target!.webView.configuration.userContentController + .addUserScript(wrapperScript); + await weakSelf.target!.webView.configuration.userContentController + .addScriptMessageHandler( + handler, + channelName, + ); + }, + ), + ); + }); } @override diff --git a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml index c69d0d51b622..56150d944213 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml +++ b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml @@ -30,3 +30,4 @@ dev_dependencies: mockito: ^5.1.0 pedantic: ^1.10.0 pigeon: ^3.0.3 + vm_service: ">=8.3.0 <10.0.0" diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index b5f7b1a486dd..966c9867d865 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:developer'; import 'dart:math'; // TODO(a14n): remove this import once Flutter 3.1 or later reaches stable (including flutter/flutter#106316) // ignore: unnecessary_import @@ -20,6 +21,26 @@ import 'package:webview_flutter_wkwebview/src/web_kit_webview_widget.dart'; import 'web_kit_webview_widget_test.mocks.dart'; +import 'dart:isolate'; +import 'package:vm_service/vm_service_io.dart' as vm_service_io; +import 'package:vm_service/vm_service.dart' as vm_service_io; + +List _cleanupPathSegments(Uri uri) { + final List pathSegments = []; + if (uri.pathSegments.isNotEmpty) { + pathSegments.addAll(uri.pathSegments.where( + (String s) => s.isNotEmpty, + )); + } + return pathSegments; +} + +String _toWebSocket(Uri uri) { + final List pathSegments = _cleanupPathSegments(uri); + pathSegments.add('ws'); + return uri.replace(scheme: 'ws', pathSegments: pathSegments).toString(); +} + @GenerateMocks([ UIScrollView, WKNavigationDelegate, @@ -34,7 +55,7 @@ import 'web_kit_webview_widget_test.mocks.dart'; WebViewPlatformCallbacksHandler, WebViewWidgetProxy, ]) -void main() { +void main() async { TestWidgetsFlutterBinding.ensureInitialized(); group('WebKitWebViewWidget', () { @@ -129,6 +150,14 @@ void main() { await tester.pumpAndSettle(); } + Future runGarbageCollection() async { + final Uri? serverUri = (await Service.getInfo()).serverUri; + final String isolateId = Service.getIsolateID(Isolate.current)!; + final vm_service_io.VmService vmService = + await vm_service_io.vmServiceConnectUri(_toWebSocket(serverUri!)); + await vmService.getAllocationProfile(isolateId, gc: true); + } + testWidgets('build $WebKitWebViewWidget', (WidgetTester tester) async { await buildWidget(tester); }); @@ -996,6 +1025,65 @@ void main() { verify(mockCallbacksHandler.onPageStarted('https://google.com')); }); + group('WebViewPlatformCallbacksHandler', () { + testWidgets('onPageStarted a', (WidgetTester tester) async { + await buildWidget(tester); + + final dynamic didStartProvisionalNavigation = + verify(mockWebViewWidgetProxy.createNavigationDelegate( + didFinishNavigation: anyNamed('didFinishNavigation'), + didStartProvisionalNavigation: + captureAnyNamed('didStartProvisionalNavigation'), + decidePolicyForNavigationAction: + anyNamed('decidePolicyForNavigationAction'), + didFailNavigation: anyNamed('didFailNavigation'), + didFailProvisionalNavigation: + anyNamed('didFailProvisionalNavigation'), + webViewWebContentProcessDidTerminate: + anyNamed('webViewWebContentProcessDidTerminate'), + )).captured.single as void Function(WKWebView, String); + didStartProvisionalNavigation(mockWebView, 'https://google.com'); + + verify(mockCallbacksHandler.onPageStarted('https://google.com')); + }); + + test('navigation delgate does not reference self', () async { + WebKitWebViewPlatformController? controller = + WebKitWebViewPlatformController( + creationParams: CreationParams( + webSettings: WebSettings( + userAgent: const WebSetting.absent(), + hasNavigationDelegate: false, + hasProgressTracking: false, + )), + callbacksHandler: mockCallbacksHandler, + javascriptChannelRegistry: mockJavascriptChannelRegistry, + webViewProxy: mockWebViewWidgetProxy, + configuration: mockWebViewConfiguration, + ); + + final dynamic didStartProvisionalNavigation = + verify(mockWebViewWidgetProxy.createNavigationDelegate( + didFinishNavigation: anyNamed('didFinishNavigation'), + didStartProvisionalNavigation: + captureAnyNamed('didStartProvisionalNavigation'), + decidePolicyForNavigationAction: + anyNamed('decidePolicyForNavigationAction'), + didFailNavigation: anyNamed('didFailNavigation'), + didFailProvisionalNavigation: + anyNamed('didFailProvisionalNavigation'), + webViewWebContentProcessDidTerminate: + anyNamed('webViewWebContentProcessDidTerminate'), + )).captured.single as void Function(WKWebView, String); + controller = null; + + await runGarbageCollection(); + + didStartProvisionalNavigation(mockWebView, 'https://google.com'); + + verify(mockCallbacksHandler.onPageStarted('https://google.com')); + }); + testWidgets('onPageFinished', (WidgetTester tester) async { await buildWidget(tester); From 308dee8a16f7ebcbf341a3cde61129f10ddc6b1b Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 09:15:57 -0700 Subject: [PATCH 04/17] use weak reference callback --- .../src/common/weak_reference_callback.dart | 14 +++ .../lib/src/common/weak_self_provider.dart | 7 -- .../lib/src/web_kit/web_kit.dart | 20 +++- .../lib/src/web_kit_webview_widget.dart | 109 +++++++++--------- 4 files changed, 87 insertions(+), 63 deletions(-) create mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart delete mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart new file mode 100644 index 000000000000..57de16d9460e --- /dev/null +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart @@ -0,0 +1,14 @@ +typedef WeakReferenceCallback = T Function(WeakReference? weakRef); + +T? passWeakReferenceToCallback( + S callback, + Object? callbackReference, +) { + if (callback == null) { + return null; + } + final WeakReference? weakReference = callbackReference == null + ? null + : WeakReference(callbackReference); + return callback(weakReference) as T; +} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart deleted file mode 100644 index 73e1468825a5..000000000000 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_self_provider.dart +++ /dev/null @@ -1,7 +0,0 @@ -/// -mixin WeakSelfProvider { - /// - S withWeakSelf(S Function(WeakReference weakSelf) onCreateWeakSelf) { - return onCreateWeakSelf(WeakReference(this as T)); - } -} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart index a671064ddc0f..80e5bae50eb0 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart @@ -6,6 +6,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; import '../common/instance_manager.dart'; +import '../common/weak_reference_callback.dart'; import '../foundation/foundation.dart'; import '../ui_kit/ui_kit.dart'; import 'web_kit_api_impls.dart'; @@ -764,7 +765,8 @@ class WKUIDelegate extends NSObject { class WKNavigationDelegate extends NSObject { /// Constructs a [WKNavigationDelegate]. WKNavigationDelegate({ - this.didFinishNavigation, + WeakReferenceCallback? + didFinishNavigation, this.didStartProvisionalNavigation, this.decidePolicyForNavigationAction, this.didFailNavigation, @@ -773,10 +775,15 @@ class WKNavigationDelegate extends NSObject { super.observeValue, super.binaryMessenger, super.instanceManager, + Object? callbackReference, }) : _navigationDelegateApi = WKNavigationDelegateHostApiImpl( binaryMessenger: binaryMessenger, instanceManager: instanceManager, ), + didFinishNavigation = passWeakReferenceToCallback( + didFinishNavigation, + callbackReference, + ), super.detached() { // Ensures FlutterApis for the WebKit library are set up. WebKitFlutterApis.instance.ensureSetUp(); @@ -789,7 +796,8 @@ class WKNavigationDelegate extends NSObject { /// This should only be used outside of tests by subclasses created by this /// library or to create a copy for an InstanceManager. WKNavigationDelegate.detached({ - this.didFinishNavigation, + WeakReferenceCallback? + didFinishNavigation, this.didStartProvisionalNavigation, this.decidePolicyForNavigationAction, this.didFailNavigation, @@ -798,10 +806,15 @@ class WKNavigationDelegate extends NSObject { super.observeValue, super.binaryMessenger, super.instanceManager, + Object? callbackReference, }) : _navigationDelegateApi = WKNavigationDelegateHostApiImpl( binaryMessenger: binaryMessenger, instanceManager: instanceManager, ), + didFinishNavigation = passWeakReferenceToCallback( + didFinishNavigation, + callbackReference, + ), super.detached(); final WKNavigationDelegateHostApiImpl _navigationDelegateApi; @@ -844,7 +857,8 @@ class WKNavigationDelegate extends NSObject { @override WKNavigationDelegate copy() { return WKNavigationDelegate.detached( - didFinishNavigation: didFinishNavigation, + didFinishNavigation: + didFinishNavigation == null ? null : ([_]) => didFinishNavigation!, didStartProvisionalNavigation: didStartProvisionalNavigation, decidePolicyForNavigationAction: decidePolicyForNavigationAction, didFailNavigation: didFailNavigation, diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index cda2d23ef95b..0a60893a96ff 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -11,6 +11,7 @@ import 'package:flutter/services.dart'; import 'package:path/path.dart' as path; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; +import 'common/weak_reference_callback.dart'; import 'common/weak_self_provider.dart'; import 'foundation/foundation.dart'; import 'web_kit/web_kit.dart'; @@ -132,8 +133,13 @@ class WebKitWebViewPlatformController extends WebViewPlatformController late final WKNavigationDelegate navigationDelegate = withWeakSelf( (WeakReference weakSelf) { return webViewProxy.createNavigationDelegate( - didFinishNavigation: (WKWebView webView, String? url) { - weakSelf.target?.callbacksHandler.onPageFinished(url ?? ''); + ref: this, + didFinishNavigation: ([WeakReference? weakRef]) { + return (WKWebView webView, String? url) { + (weakRef!.target as WebKitWebViewPlatformController?) + ?.callbacksHandler + .onPageFinished(url ?? ''); + }; }, didStartProvisionalNavigation: (WKWebView webView, String? url) { weakSelf.target?.callbacksHandler.onPageStarted(url ?? ''); @@ -420,54 +426,52 @@ class WebKitWebViewPlatformController extends WebViewPlatformController @override Future addJavascriptChannels(Set javascriptChannelNames) async { - await withWeakSelf(( - WeakReference weakSelf, - ) { - return Future.wait( - javascriptChannelNames.where( - (String channelName) { - if (weakSelf.target == null) { - return false; - } - return !weakSelf.target!._scriptMessageHandlers - .containsKey(channelName); - }, - ).map>( - (String channelName) async { - if (weakSelf.target == null) { - return; - } - final WKScriptMessageHandler handler = weakSelf.target!.webViewProxy - .createScriptMessageHandler(didReceiveScriptMessage: ( - WKUserContentController userContentController, - WKScriptMessage message, - ) { - weakSelf.target!.javascriptChannelRegistry - .onJavascriptChannelMessage( - message.name, - message.body!.toString(), - ); - }); - weakSelf.target!._scriptMessageHandlers[channelName] = handler; - - final String wrapperSource = - 'window.$channelName = webkit.messageHandlers.$channelName;'; - final WKUserScript wrapperScript = WKUserScript( - wrapperSource, - WKUserScriptInjectionTime.atDocumentStart, - isMainFrameOnly: false, - ); - weakSelf.target!.webView.configuration.userContentController - .addUserScript(wrapperScript); - await weakSelf.target!.webView.configuration.userContentController - .addScriptMessageHandler( - handler, - channelName, + final WeakReference weakSelf = + WeakReference(this); + return Future.wait( + javascriptChannelNames.where( + (String channelName) { + if (weakSelf.target == null) { + return false; + } + return !weakSelf.target!._scriptMessageHandlers + .containsKey(channelName); + }, + ).map>( + (String channelName) async { + if (weakSelf.target == null) { + return; + } + final WKScriptMessageHandler handler = weakSelf.target!.webViewProxy + .createScriptMessageHandler(didReceiveScriptMessage: ( + WKUserContentController userContentController, + WKScriptMessage message, + ) { + weakSelf.target!.javascriptChannelRegistry + .onJavascriptChannelMessage( + message.name, + message.body!.toString(), ); - }, - ), - ); - }); + }); + weakSelf.target!._scriptMessageHandlers[channelName] = handler; + + final String wrapperSource = + 'window.$channelName = webkit.messageHandlers.$channelName;'; + final WKUserScript wrapperScript = WKUserScript( + wrapperSource, + WKUserScriptInjectionTime.atDocumentStart, + isMainFrameOnly: false, + ); + weakSelf.target!.webView.configuration.userContentController + .addUserScript(wrapperScript); + await weakSelf.target!.webView.configuration.userContentController + .addScriptMessageHandler( + handler, + channelName, + ); + }, + ), + ); } @override @@ -679,10 +683,7 @@ class WebViewWidgetProxy { /// Constructs a [WKNavigationDelegate]. WKNavigationDelegate createNavigationDelegate({ - void Function( - WKWebView webView, - String? url, - )? + WeakReferenceCallback? didFinishNavigation, void Function(WKWebView webView, String? url)? didStartProvisionalNavigation, @@ -695,6 +696,7 @@ class WebViewWidgetProxy { void Function(WKWebView webView, NSError error)? didFailProvisionalNavigation, void Function(WKWebView webView)? webViewWebContentProcessDidTerminate, + Object? callbackReference, }) { return WKNavigationDelegate( didFinishNavigation: didFinishNavigation, @@ -704,6 +706,7 @@ class WebViewWidgetProxy { didFailProvisionalNavigation: didFailProvisionalNavigation, webViewWebContentProcessDidTerminate: webViewWebContentProcessDidTerminate, + callbackReference: callbackReference, ); } } From 6731367550e9386314fa89c4903809bf769931b7 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 10:39:46 -0700 Subject: [PATCH 05/17] create a weakref helper method --- .../src/common/weak_reference_callback.dart | 14 -- .../lib/src/common/weak_reference_utils.dart | 7 + .../lib/src/web_kit/web_kit.dart | 20 +- .../lib/src/web_kit_webview_widget.dart | 216 +++++++++--------- 4 files changed, 116 insertions(+), 141 deletions(-) delete mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart create mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart deleted file mode 100644 index 57de16d9460e..000000000000 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_callback.dart +++ /dev/null @@ -1,14 +0,0 @@ -typedef WeakReferenceCallback = T Function(WeakReference? weakRef); - -T? passWeakReferenceToCallback( - S callback, - Object? callbackReference, -) { - if (callback == null) { - return null; - } - final WeakReference? weakReference = callbackReference == null - ? null - : WeakReference(callbackReference); - return callback(weakReference) as T; -} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart new file mode 100644 index 000000000000..e590ecacedd9 --- /dev/null +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart @@ -0,0 +1,7 @@ +/// Helper method to create a callback with a [WeakReference]. +S withWeakRef( + T reference, + S Function(WeakReference weakRef) onCreateCallback, +) { + return onCreateCallback(WeakReference(reference)); +} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart index 80e5bae50eb0..a671064ddc0f 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart @@ -6,7 +6,6 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; import '../common/instance_manager.dart'; -import '../common/weak_reference_callback.dart'; import '../foundation/foundation.dart'; import '../ui_kit/ui_kit.dart'; import 'web_kit_api_impls.dart'; @@ -765,8 +764,7 @@ class WKUIDelegate extends NSObject { class WKNavigationDelegate extends NSObject { /// Constructs a [WKNavigationDelegate]. WKNavigationDelegate({ - WeakReferenceCallback? - didFinishNavigation, + this.didFinishNavigation, this.didStartProvisionalNavigation, this.decidePolicyForNavigationAction, this.didFailNavigation, @@ -775,15 +773,10 @@ class WKNavigationDelegate extends NSObject { super.observeValue, super.binaryMessenger, super.instanceManager, - Object? callbackReference, }) : _navigationDelegateApi = WKNavigationDelegateHostApiImpl( binaryMessenger: binaryMessenger, instanceManager: instanceManager, ), - didFinishNavigation = passWeakReferenceToCallback( - didFinishNavigation, - callbackReference, - ), super.detached() { // Ensures FlutterApis for the WebKit library are set up. WebKitFlutterApis.instance.ensureSetUp(); @@ -796,8 +789,7 @@ class WKNavigationDelegate extends NSObject { /// This should only be used outside of tests by subclasses created by this /// library or to create a copy for an InstanceManager. WKNavigationDelegate.detached({ - WeakReferenceCallback? - didFinishNavigation, + this.didFinishNavigation, this.didStartProvisionalNavigation, this.decidePolicyForNavigationAction, this.didFailNavigation, @@ -806,15 +798,10 @@ class WKNavigationDelegate extends NSObject { super.observeValue, super.binaryMessenger, super.instanceManager, - Object? callbackReference, }) : _navigationDelegateApi = WKNavigationDelegateHostApiImpl( binaryMessenger: binaryMessenger, instanceManager: instanceManager, ), - didFinishNavigation = passWeakReferenceToCallback( - didFinishNavigation, - callbackReference, - ), super.detached(); final WKNavigationDelegateHostApiImpl _navigationDelegateApi; @@ -857,8 +844,7 @@ class WKNavigationDelegate extends NSObject { @override WKNavigationDelegate copy() { return WKNavigationDelegate.detached( - didFinishNavigation: - didFinishNavigation == null ? null : ([_]) => didFinishNavigation!, + didFinishNavigation: didFinishNavigation, didStartProvisionalNavigation: didStartProvisionalNavigation, decidePolicyForNavigationAction: decidePolicyForNavigationAction, didFailNavigation: didFailNavigation, diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index 0a60893a96ff..e5f5b7b10deb 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -11,8 +11,7 @@ import 'package:flutter/services.dart'; import 'package:path/path.dart' as path; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; -import 'common/weak_reference_callback.dart'; -import 'common/weak_self_provider.dart'; +import 'common/weak_reference_utils.dart'; import 'foundation/foundation.dart'; import 'web_kit/web_kit.dart'; @@ -78,8 +77,7 @@ class _WebKitWebViewWidgetState extends State { } /// An implementation of [WebViewPlatformController] with the WebKit api. -class WebKitWebViewPlatformController extends WebViewPlatformController - with WeakSelfProvider { +class WebKitWebViewPlatformController extends WebViewPlatformController { /// Construct a [WebKitWebViewPlatformController]. WebKitWebViewPlatformController({ required CreationParams creationParams, @@ -117,74 +115,72 @@ class WebKitWebViewPlatformController extends WebViewPlatformController /// Used to integrate custom user interface elements into web view interactions. @visibleForTesting - late final WKUIDelegate uiDelegate = - webViewProxy.createUIDelgate(onCreateWebView: ( - WKWebView webView, - WKWebViewConfiguration configuration, - WKNavigationAction navigationAction, - ) { - if (!navigationAction.targetFrame.isMainFrame) { - webView.loadRequest(navigationAction.request); - } - }); + late final WKUIDelegate uiDelegate = webViewProxy.createUIDelgate( + onCreateWebView: ( + WKWebView webView, + WKWebViewConfiguration configuration, + WKNavigationAction navigationAction, + ) { + if (!navigationAction.targetFrame.isMainFrame) { + webView.loadRequest(navigationAction.request); + } + }, + ); /// Methods for handling navigation changes and tracking navigation requests. @visibleForTesting - late final WKNavigationDelegate navigationDelegate = withWeakSelf( - (WeakReference weakSelf) { - return webViewProxy.createNavigationDelegate( - ref: this, - didFinishNavigation: ([WeakReference? weakRef]) { - return (WKWebView webView, String? url) { - (weakRef!.target as WebKitWebViewPlatformController?) - ?.callbacksHandler - .onPageFinished(url ?? ''); - }; - }, - didStartProvisionalNavigation: (WKWebView webView, String? url) { - weakSelf.target?.callbacksHandler.onPageStarted(url ?? ''); - }, - decidePolicyForNavigationAction: ( - WKWebView webView, - WKNavigationAction action, - ) async { - if (weakSelf.target == null) { - return WKNavigationActionPolicy.allow; - } - if (!weakSelf.target!._hasNavigationDelegate) { - return WKNavigationActionPolicy.allow; - } - - final bool allow = - await weakSelf.target!.callbacksHandler.onNavigationRequest( - url: action.request.url, - isForMainFrame: action.targetFrame.isMainFrame, - ); + late final WKNavigationDelegate navigationDelegate = withWeakRef(this, + (WeakReference weakRef) { + return webViewProxy.createNavigationDelegate( + didFinishNavigation: (WKWebView webView, String? url) { + weakRef.target?.callbacksHandler.onPageFinished(url ?? ''); + }, + didStartProvisionalNavigation: (WKWebView webView, String? url) { + weakRef.target?.callbacksHandler.onPageStarted(url ?? ''); + }, + decidePolicyForNavigationAction: ( + WKWebView webView, + WKNavigationAction action, + ) async { + if (weakRef.target == null) { + return WKNavigationActionPolicy.allow; + } + + if (!weakRef.target!._hasNavigationDelegate) { + return WKNavigationActionPolicy.allow; + } + + final bool allow = + await weakRef.target!.callbacksHandler.onNavigationRequest( + url: action.request.url, + isForMainFrame: action.targetFrame.isMainFrame, + ); - return allow - ? WKNavigationActionPolicy.allow - : WKNavigationActionPolicy.cancel; - }, - didFailNavigation: (WKWebView webView, NSError error) { - weakSelf.target?.callbacksHandler - .onWebResourceError(_toWebResourceError(error)); - }, - didFailProvisionalNavigation: (WKWebView webView, NSError error) { - weakSelf.target?.callbacksHandler - .onWebResourceError(_toWebResourceError(error)); - }, - webViewWebContentProcessDidTerminate: (WKWebView webView) { - weakSelf.target?.callbacksHandler.onWebResourceError(WebResourceError( - errorCode: WKErrorCode.webContentProcessTerminated, - // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. - domain: 'WKErrorDomain', - description: '', - errorType: WebResourceErrorType.webContentProcessTerminated, - )); - }, - ); - }, - ); + return allow + ? WKNavigationActionPolicy.allow + : WKNavigationActionPolicy.cancel; + }, + didFailNavigation: (WKWebView webView, NSError error) { + weakRef.target?.callbacksHandler.onWebResourceError( + _toWebResourceError(error), + ); + }, + didFailProvisionalNavigation: (WKWebView webView, NSError error) { + weakRef.target?.callbacksHandler.onWebResourceError( + _toWebResourceError(error), + ); + }, + webViewWebContentProcessDidTerminate: (WKWebView webView) { + weakRef.target?.callbacksHandler.onWebResourceError(WebResourceError( + errorCode: WKErrorCode.webContentProcessTerminated, + // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. + domain: 'WKErrorDomain', + description: '', + errorType: WebResourceErrorType.webContentProcessTerminated, + )); + }, + ); + }); Future _setCreationParams( CreationParams params, { @@ -196,18 +192,23 @@ class WebKitWebViewPlatformController extends WebViewPlatformController autoMediaPlaybackPolicy: params.autoMediaPlaybackPolicy, ); - webView = withWeakSelf(( - WeakReference weakSelf, - ) { - return webViewProxy.createWebView(configuration, observeValue: ( - String keyPath, - NSObject object, - Map change, - ) { - final double progress = change[NSKeyValueChangeKey.newValue]! as double; - weakSelf.target?.callbacksHandler.onProgress((progress * 100).round()); - }); - }); + webView = webViewProxy.createWebView( + configuration, + observeValue: withWeakRef( + callbacksHandler, + (WeakReference weakRef) { + return ( + String keyPath, + NSObject object, + Map change, + ) { + final double progress = + change[NSKeyValueChangeKey.newValue]! as double; + weakRef.target?.onProgress((progress * 100).round()); + }; + }, + ), + ); webView.setUIDelegate(uiDelegate); @@ -426,34 +427,31 @@ class WebKitWebViewPlatformController extends WebViewPlatformController @override Future addJavascriptChannels(Set javascriptChannelNames) async { - final WeakReference weakSelf = - WeakReference(this); - return Future.wait( + await Future.wait( javascriptChannelNames.where( (String channelName) { - if (weakSelf.target == null) { - return false; - } - return !weakSelf.target!._scriptMessageHandlers - .containsKey(channelName); + return !_scriptMessageHandlers.containsKey(channelName); }, ).map>( - (String channelName) async { - if (weakSelf.target == null) { - return; - } - final WKScriptMessageHandler handler = weakSelf.target!.webViewProxy - .createScriptMessageHandler(didReceiveScriptMessage: ( - WKUserContentController userContentController, - WKScriptMessage message, - ) { - weakSelf.target!.javascriptChannelRegistry - .onJavascriptChannelMessage( - message.name, - message.body!.toString(), - ); - }); - weakSelf.target!._scriptMessageHandlers[channelName] = handler; + (String channelName) { + final WKScriptMessageHandler handler = + webViewProxy.createScriptMessageHandler( + didReceiveScriptMessage: withWeakRef( + javascriptChannelRegistry, + (WeakReference weakRef) { + return ( + WKUserContentController userContentController, + WKScriptMessage message, + ) { + weakRef.target?.onJavascriptChannelMessage( + message.name, + message.body!.toString(), + ); + }; + }, + ), + ); + _scriptMessageHandlers[channelName] = handler; final String wrapperSource = 'window.$channelName = webkit.messageHandlers.$channelName;'; @@ -462,9 +460,9 @@ class WebKitWebViewPlatformController extends WebViewPlatformController WKUserScriptInjectionTime.atDocumentStart, isMainFrameOnly: false, ); - weakSelf.target!.webView.configuration.userContentController + webView.configuration.userContentController .addUserScript(wrapperScript); - await weakSelf.target!.webView.configuration.userContentController + return webView.configuration.userContentController .addScriptMessageHandler( handler, channelName, @@ -671,6 +669,7 @@ class WebViewWidgetProxy { /// Constructs a [WKUIDelegate]. WKUIDelegate createUIDelgate({ + Object? callbackReference, void Function( WKWebView webView, WKWebViewConfiguration configuration, @@ -683,8 +682,7 @@ class WebViewWidgetProxy { /// Constructs a [WKNavigationDelegate]. WKNavigationDelegate createNavigationDelegate({ - WeakReferenceCallback? - didFinishNavigation, + void Function(WKWebView webView, String? url)? didFinishNavigation, void Function(WKWebView webView, String? url)? didStartProvisionalNavigation, Future Function( @@ -696,7 +694,6 @@ class WebViewWidgetProxy { void Function(WKWebView webView, NSError error)? didFailProvisionalNavigation, void Function(WKWebView webView)? webViewWebContentProcessDidTerminate, - Object? callbackReference, }) { return WKNavigationDelegate( didFinishNavigation: didFinishNavigation, @@ -706,7 +703,6 @@ class WebViewWidgetProxy { didFailProvisionalNavigation: didFailProvisionalNavigation, webViewWebContentProcessDidTerminate: webViewWebContentProcessDidTerminate, - callbackReference: callbackReference, ); } } From 96077cb04f905657557c6d028d7e8e4cc27ff7c5 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 11:34:11 -0700 Subject: [PATCH 06/17] tests running but failing --- .../lib/src/web_kit_webview_widget.dart | 1 - .../test/src/web_kit_webview_widget_test.dart | 106 +++++++----------- 2 files changed, 42 insertions(+), 65 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index e5f5b7b10deb..8db71f6a317b 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -669,7 +669,6 @@ class WebViewWidgetProxy { /// Constructs a [WKUIDelegate]. WKUIDelegate createUIDelgate({ - Object? callbackReference, void Function( WKWebView webView, WKWebViewConfiguration configuration, diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index 966c9867d865..223eca65b4c5 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:developer'; +import 'dart:isolate'; import 'dart:math'; // TODO(a14n): remove this import once Flutter 3.1 or later reaches stable (including flutter/flutter#106316) // ignore: unnecessary_import @@ -13,6 +14,8 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; +import 'package:vm_service/vm_service.dart' as vm_service; +import 'package:vm_service/vm_service_io.dart' as vm_service_io; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; import 'package:webview_flutter_wkwebview/src/foundation/foundation.dart'; import 'package:webview_flutter_wkwebview/src/ui_kit/ui_kit.dart'; @@ -21,10 +24,6 @@ import 'package:webview_flutter_wkwebview/src/web_kit_webview_widget.dart'; import 'web_kit_webview_widget_test.mocks.dart'; -import 'dart:isolate'; -import 'package:vm_service/vm_service_io.dart' as vm_service_io; -import 'package:vm_service/vm_service.dart' as vm_service_io; - List _cleanupPathSegments(Uri uri) { final List pathSegments = []; if (uri.pathSegments.isNotEmpty) { @@ -153,7 +152,7 @@ void main() async { Future runGarbageCollection() async { final Uri? serverUri = (await Service.getInfo()).serverUri; final String isolateId = Service.getIsolateID(Isolate.current)!; - final vm_service_io.VmService vmService = + final vm_service.VmService vmService = await vm_service_io.vmServiceConnectUri(_toWebSocket(serverUri!)); await vmService.getAllocationProfile(isolateId, gc: true); } @@ -187,6 +186,44 @@ void main() async { verify(mockWebView.loadRequest(request)); }); + test('callback methods do not cause a self reference', () async { + WebKitWebViewPlatformController? controller = + WebKitWebViewPlatformController( + creationParams: CreationParams( + webSettings: WebSettings( + userAgent: const WebSetting.absent(), + hasNavigationDelegate: false, + hasProgressTracking: false, + )), + callbacksHandler: mockCallbacksHandler, + javascriptChannelRegistry: mockJavascriptChannelRegistry, + webViewProxy: mockWebViewWidgetProxy, + configuration: mockWebViewConfiguration, + ); + + controller.navigationDelegate; + + final dynamic didStartProvisionalNavigation = + verify(mockWebViewWidgetProxy.createNavigationDelegate( + didFinishNavigation: anyNamed('didFinishNavigation'), + didStartProvisionalNavigation: + captureAnyNamed('didStartProvisionalNavigation'), + decidePolicyForNavigationAction: + anyNamed('decidePolicyForNavigationAction'), + didFailNavigation: anyNamed('didFailNavigation'), + didFailProvisionalNavigation: anyNamed('didFailProvisionalNavigation'), + webViewWebContentProcessDidTerminate: + anyNamed('webViewWebContentProcessDidTerminate'), + )).captured.single as void Function(WKWebView, String); + controller = null; + + await runGarbageCollection(); + + didStartProvisionalNavigation(mockWebView, 'https://google.com'); + + verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); + }); + group('CreationParams', () { testWidgets('initialUrl', (WidgetTester tester) async { await buildWidget( @@ -1025,65 +1062,6 @@ void main() async { verify(mockCallbacksHandler.onPageStarted('https://google.com')); }); - group('WebViewPlatformCallbacksHandler', () { - testWidgets('onPageStarted a', (WidgetTester tester) async { - await buildWidget(tester); - - final dynamic didStartProvisionalNavigation = - verify(mockWebViewWidgetProxy.createNavigationDelegate( - didFinishNavigation: anyNamed('didFinishNavigation'), - didStartProvisionalNavigation: - captureAnyNamed('didStartProvisionalNavigation'), - decidePolicyForNavigationAction: - anyNamed('decidePolicyForNavigationAction'), - didFailNavigation: anyNamed('didFailNavigation'), - didFailProvisionalNavigation: - anyNamed('didFailProvisionalNavigation'), - webViewWebContentProcessDidTerminate: - anyNamed('webViewWebContentProcessDidTerminate'), - )).captured.single as void Function(WKWebView, String); - didStartProvisionalNavigation(mockWebView, 'https://google.com'); - - verify(mockCallbacksHandler.onPageStarted('https://google.com')); - }); - - test('navigation delgate does not reference self', () async { - WebKitWebViewPlatformController? controller = - WebKitWebViewPlatformController( - creationParams: CreationParams( - webSettings: WebSettings( - userAgent: const WebSetting.absent(), - hasNavigationDelegate: false, - hasProgressTracking: false, - )), - callbacksHandler: mockCallbacksHandler, - javascriptChannelRegistry: mockJavascriptChannelRegistry, - webViewProxy: mockWebViewWidgetProxy, - configuration: mockWebViewConfiguration, - ); - - final dynamic didStartProvisionalNavigation = - verify(mockWebViewWidgetProxy.createNavigationDelegate( - didFinishNavigation: anyNamed('didFinishNavigation'), - didStartProvisionalNavigation: - captureAnyNamed('didStartProvisionalNavigation'), - decidePolicyForNavigationAction: - anyNamed('decidePolicyForNavigationAction'), - didFailNavigation: anyNamed('didFailNavigation'), - didFailProvisionalNavigation: - anyNamed('didFailProvisionalNavigation'), - webViewWebContentProcessDidTerminate: - anyNamed('webViewWebContentProcessDidTerminate'), - )).captured.single as void Function(WKWebView, String); - controller = null; - - await runGarbageCollection(); - - didStartProvisionalNavigation(mockWebView, 'https://google.com'); - - verify(mockCallbacksHandler.onPageStarted('https://google.com')); - }); - testWidgets('onPageFinished', (WidgetTester tester) async { await buildWidget(tester); From f468f2ce054c01d86e72914ce3d4cec371269f7b Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 15:37:58 -0700 Subject: [PATCH 07/17] some test progress --- .../lib/src/common/weak_reference_utils.dart | 7 - .../lib/src/web_kit_webview_widget.dart | 145 +++++++++--------- .../test/src/web_kit_webview_widget_test.dart | 98 +++++++++++- 3 files changed, 163 insertions(+), 87 deletions(-) delete mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart deleted file mode 100644 index e590ecacedd9..000000000000 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart +++ /dev/null @@ -1,7 +0,0 @@ -/// Helper method to create a callback with a [WeakReference]. -S withWeakRef( - T reference, - S Function(WeakReference weakRef) onCreateCallback, -) { - return onCreateCallback(WeakReference(reference)); -} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index 8db71f6a317b..ffe32dd3dcea 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -11,7 +11,6 @@ import 'package:flutter/services.dart'; import 'package:path/path.dart' as path; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; -import 'common/weak_reference_utils.dart'; import 'foundation/foundation.dart'; import 'web_kit/web_kit.dart'; @@ -86,52 +85,9 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { WKWebViewConfiguration? configuration, @visibleForTesting this.webViewProxy = const WebViewWidgetProxy(), }) : super(callbacksHandler) { - _setCreationParams( - creationParams, - configuration: configuration ?? WKWebViewConfiguration(), - ); - } - - bool _zoomEnabled = true; - bool _hasNavigationDelegate = false; - bool _progressObserverSet = false; - - final Map _scriptMessageHandlers = - {}; - - /// Handles callbacks that are made by navigation. - final WebViewPlatformCallbacksHandler callbacksHandler; - - /// Manages named JavaScript channels and forwarding incoming messages on the correct channel. - final JavascriptChannelRegistry javascriptChannelRegistry; - - /// Handles constructing a [WKWebView]. - /// - /// This should only be changed when used for testing. - final WebViewWidgetProxy webViewProxy; - - /// Represents the WebView maintained by platform code. - late final WKWebView webView; - - /// Used to integrate custom user interface elements into web view interactions. - @visibleForTesting - late final WKUIDelegate uiDelegate = webViewProxy.createUIDelgate( - onCreateWebView: ( - WKWebView webView, - WKWebViewConfiguration configuration, - WKNavigationAction navigationAction, - ) { - if (!navigationAction.targetFrame.isMainFrame) { - webView.loadRequest(navigationAction.request); - } - }, - ); - - /// Methods for handling navigation changes and tracking navigation requests. - @visibleForTesting - late final WKNavigationDelegate navigationDelegate = withWeakRef(this, - (WeakReference weakRef) { - return webViewProxy.createNavigationDelegate( + final WeakReference weakRef = + WeakReference(this); + navigationDelegate = webViewProxy.createNavigationDelegate( didFinishNavigation: (WKWebView webView, String? url) { weakRef.target?.callbacksHandler.onPageFinished(url ?? ''); }, @@ -180,7 +136,49 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { )); }, ); - }); + + _setCreationParams( + creationParams, + configuration: configuration ?? WKWebViewConfiguration(), + ); + } + + bool _zoomEnabled = true; + bool _hasNavigationDelegate = false; + bool _progressObserverSet = false; + + final Map _scriptMessageHandlers = + {}; + + /// Handles callbacks that are made by navigation. + final WebViewPlatformCallbacksHandler callbacksHandler; + + /// Manages named JavaScript channels and forwarding incoming messages on the correct channel. + final JavascriptChannelRegistry javascriptChannelRegistry; + + /// Handles constructing a [WKWebView]. + /// + /// This should only be changed when used for testing. + final WebViewWidgetProxy webViewProxy; + + /// Represents the WebView maintained by platform code. + late final WKWebView webView; + + /// Used to integrate custom user interface elements into web view interactions. + @visibleForTesting + late final WKUIDelegate uiDelegate = webViewProxy.createUIDelgate( + onCreateWebView: ( + WKWebView webView, + WKWebViewConfiguration configuration, + WKNavigationAction navigationAction, + ) { + if (!navigationAction.targetFrame.isMainFrame) { + webView.loadRequest(navigationAction.request); + } + }, + ); + + late final WKNavigationDelegate navigationDelegate; Future _setCreationParams( CreationParams params, { @@ -192,22 +190,18 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { autoMediaPlaybackPolicy: params.autoMediaPlaybackPolicy, ); + final WeakReference weakRef = + WeakReference(callbacksHandler); webView = webViewProxy.createWebView( configuration, - observeValue: withWeakRef( - callbacksHandler, - (WeakReference weakRef) { - return ( - String keyPath, - NSObject object, - Map change, - ) { - final double progress = - change[NSKeyValueChangeKey.newValue]! as double; - weakRef.target?.onProgress((progress * 100).round()); - }; - }, - ), + observeValue: ( + String keyPath, + NSObject object, + Map change, + ) { + final double progress = change[NSKeyValueChangeKey.newValue]! as double; + weakRef.target?.onProgress((progress * 100).round()); + }, ); webView.setUIDelegate(uiDelegate); @@ -427,6 +421,10 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { @override Future addJavascriptChannels(Set javascriptChannelNames) async { + final WeakReference weakRef = + WeakReference( + javascriptChannelRegistry, + ); await Future.wait( javascriptChannelNames.where( (String channelName) { @@ -436,20 +434,15 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { (String channelName) { final WKScriptMessageHandler handler = webViewProxy.createScriptMessageHandler( - didReceiveScriptMessage: withWeakRef( - javascriptChannelRegistry, - (WeakReference weakRef) { - return ( - WKUserContentController userContentController, - WKScriptMessage message, - ) { - weakRef.target?.onJavascriptChannelMessage( - message.name, - message.body!.toString(), - ); - }; - }, - ), + didReceiveScriptMessage: ( + WKUserContentController userContentController, + WKScriptMessage message, + ) { + weakRef.target?.onJavascriptChannelMessage( + message.name, + message.body!.toString(), + ); + }, ); _scriptMessageHandlers[channelName] = handler; diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index 223eca65b4c5..c8e18c474e6c 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -186,7 +186,10 @@ void main() async { verify(mockWebView.loadRequest(request)); }); - test('callback methods do not cause a self reference', () async { + test( + 'navigation delegate callback methods do not cause a circular reference', + () async { + // ignore: unused_local_variable WebKitWebViewPlatformController? controller = WebKitWebViewPlatformController( creationParams: CreationParams( @@ -201,9 +204,7 @@ void main() async { configuration: mockWebViewConfiguration, ); - controller.navigationDelegate; - - final dynamic didStartProvisionalNavigation = + final void Function(WKWebView, String) didStartProvisionalNavigation = verify(mockWebViewWidgetProxy.createNavigationDelegate( didFinishNavigation: anyNamed('didFinishNavigation'), didStartProvisionalNavigation: @@ -215,12 +216,101 @@ void main() async { webViewWebContentProcessDidTerminate: anyNamed('webViewWebContentProcessDidTerminate'), )).captured.single as void Function(WKWebView, String); + controller = null; + await runGarbageCollection(); + + didStartProvisionalNavigation(mockWebView, 'https://google.com'); + verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); + }); + test('uidelgate callback methods do not cause a circular reference', + () async { + // ignore: unused_local_variable + WebKitWebViewPlatformController? controller = + WebKitWebViewPlatformController( + creationParams: CreationParams( + webSettings: WebSettings( + userAgent: const WebSetting.absent(), + hasNavigationDelegate: false, + hasProgressTracking: false, + )), + callbacksHandler: mockCallbacksHandler, + javascriptChannelRegistry: mockJavascriptChannelRegistry, + webViewProxy: mockWebViewWidgetProxy, + configuration: mockWebViewConfiguration, + ); + + // Lazy initialization. + // ignore: unnecessary_statements + controller.uiDelegate; + + final void Function(WKWebView, String) didStartProvisionalNavigation = + verify(mockWebViewWidgetProxy.createNavigationDelegate( + didFinishNavigation: anyNamed('didFinishNavigation'), + didStartProvisionalNavigation: + captureAnyNamed('didStartProvisionalNavigation'), + decidePolicyForNavigationAction: + anyNamed('decidePolicyForNavigationAction'), + didFailNavigation: anyNamed('didFailNavigation'), + didFailProvisionalNavigation: anyNamed('didFailProvisionalNavigation'), + webViewWebContentProcessDidTerminate: + anyNamed('webViewWebContentProcessDidTerminate'), + )).captured.single as void Function(WKWebView, String); + + controller = null; await runGarbageCollection(); didStartProvisionalNavigation(mockWebView, 'https://google.com'); + verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); + }); + test( + 'ScriptMessageHandler callback method does not cause a circular reference', + () async { + // ignore: unused_local_variable + WebKitWebViewPlatformController? controller = + WebKitWebViewPlatformController( + creationParams: CreationParams( + webSettings: WebSettings( + userAgent: const WebSetting.absent(), + hasNavigationDelegate: false, + hasProgressTracking: false, + )), + callbacksHandler: mockCallbacksHandler, + javascriptChannelRegistry: mockJavascriptChannelRegistry, + webViewProxy: mockWebViewWidgetProxy, + configuration: mockWebViewConfiguration, + ); + + when( + mockWebViewWidgetProxy.createScriptMessageHandler( + didReceiveScriptMessage: anyNamed('didReceiveScriptMessage'), + ), + ).thenReturn(MockWKScriptMessageHandler()); + + await controller.addJavascriptChannels({'oijw'}); + + final void Function(WKWebView, String) didStartProvisionalNavigation = + verify(mockWebViewWidgetProxy.createNavigationDelegate( + didFinishNavigation: anyNamed('didFinishNavigation'), + didStartProvisionalNavigation: + captureAnyNamed('didStartProvisionalNavigation'), + decidePolicyForNavigationAction: + anyNamed('decidePolicyForNavigationAction'), + didFailNavigation: anyNamed('didFailNavigation'), + didFailProvisionalNavigation: anyNamed('didFailProvisionalNavigation'), + webViewWebContentProcessDidTerminate: + anyNamed('webViewWebContentProcessDidTerminate'), + )).captured.single as void Function(WKWebView, String); + + controller = null; + await runGarbageCollection(); + await runGarbageCollection(); + await runGarbageCollection(); + await runGarbageCollection(); + + didStartProvisionalNavigation(mockWebView, 'https://google.com'); verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); }); From 6596bd714c3d03f20d93b26f2f388e1c345736c8 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 16:31:05 -0700 Subject: [PATCH 08/17] use weak reference helper --- .../lib/src/common/weak_reference_utils.dart | 8 + .../lib/src/web_kit_webview_widget.dart | 163 ++++++++++-------- .../test/src/web_kit_webview_widget_test.dart | 128 -------------- 3 files changed, 95 insertions(+), 204 deletions(-) create mode 100644 packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart new file mode 100644 index 000000000000..5e7a871c7792 --- /dev/null +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart @@ -0,0 +1,8 @@ +/// Helper method for creating callbacks methods with a weak reference. +S withWeakRefenceTo( + T reference, + S Function(WeakReference weakReference) onCreate, +) { + final WeakReference weakReference = WeakReference(reference); + return onCreate(weakReference); +} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart index ffe32dd3dcea..852f9caa0c49 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart @@ -11,6 +11,7 @@ import 'package:flutter/services.dart'; import 'package:path/path.dart' as path; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; +import 'common/weak_reference_utils.dart'; import 'foundation/foundation.dart'; import 'web_kit/web_kit.dart'; @@ -85,58 +86,6 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { WKWebViewConfiguration? configuration, @visibleForTesting this.webViewProxy = const WebViewWidgetProxy(), }) : super(callbacksHandler) { - final WeakReference weakRef = - WeakReference(this); - navigationDelegate = webViewProxy.createNavigationDelegate( - didFinishNavigation: (WKWebView webView, String? url) { - weakRef.target?.callbacksHandler.onPageFinished(url ?? ''); - }, - didStartProvisionalNavigation: (WKWebView webView, String? url) { - weakRef.target?.callbacksHandler.onPageStarted(url ?? ''); - }, - decidePolicyForNavigationAction: ( - WKWebView webView, - WKNavigationAction action, - ) async { - if (weakRef.target == null) { - return WKNavigationActionPolicy.allow; - } - - if (!weakRef.target!._hasNavigationDelegate) { - return WKNavigationActionPolicy.allow; - } - - final bool allow = - await weakRef.target!.callbacksHandler.onNavigationRequest( - url: action.request.url, - isForMainFrame: action.targetFrame.isMainFrame, - ); - - return allow - ? WKNavigationActionPolicy.allow - : WKNavigationActionPolicy.cancel; - }, - didFailNavigation: (WKWebView webView, NSError error) { - weakRef.target?.callbacksHandler.onWebResourceError( - _toWebResourceError(error), - ); - }, - didFailProvisionalNavigation: (WKWebView webView, NSError error) { - weakRef.target?.callbacksHandler.onWebResourceError( - _toWebResourceError(error), - ); - }, - webViewWebContentProcessDidTerminate: (WKWebView webView) { - weakRef.target?.callbacksHandler.onWebResourceError(WebResourceError( - errorCode: WKErrorCode.webContentProcessTerminated, - // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. - domain: 'WKErrorDomain', - description: '', - errorType: WebResourceErrorType.webContentProcessTerminated, - )); - }, - ); - _setCreationParams( creationParams, configuration: configuration ?? WKWebViewConfiguration(), @@ -178,7 +127,64 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { }, ); - late final WKNavigationDelegate navigationDelegate; + /// Methods for handling navigation changes and tracking navigation requests. + @visibleForTesting + late final WKNavigationDelegate navigationDelegate = withWeakRefenceTo( + this, + (WeakReference weakReference) { + return webViewProxy.createNavigationDelegate( + didFinishNavigation: (WKWebView webView, String? url) { + weakReference.target?.callbacksHandler.onPageFinished(url ?? ''); + }, + didStartProvisionalNavigation: (WKWebView webView, String? url) { + weakReference.target?.callbacksHandler.onPageStarted(url ?? ''); + }, + decidePolicyForNavigationAction: ( + WKWebView webView, + WKNavigationAction action, + ) async { + if (weakReference.target == null) { + return WKNavigationActionPolicy.allow; + } + + if (!weakReference.target!._hasNavigationDelegate) { + return WKNavigationActionPolicy.allow; + } + + final bool allow = + await weakReference.target!.callbacksHandler.onNavigationRequest( + url: action.request.url, + isForMainFrame: action.targetFrame.isMainFrame, + ); + + return allow + ? WKNavigationActionPolicy.allow + : WKNavigationActionPolicy.cancel; + }, + didFailNavigation: (WKWebView webView, NSError error) { + weakReference.target?.callbacksHandler.onWebResourceError( + _toWebResourceError(error), + ); + }, + didFailProvisionalNavigation: (WKWebView webView, NSError error) { + weakReference.target?.callbacksHandler.onWebResourceError( + _toWebResourceError(error), + ); + }, + webViewWebContentProcessDidTerminate: (WKWebView webView) { + weakReference.target?.callbacksHandler.onWebResourceError( + WebResourceError( + errorCode: WKErrorCode.webContentProcessTerminated, + // Value from https://developer.apple.com/documentation/webkit/wkerrordomain?language=objc. + domain: 'WKErrorDomain', + description: '', + errorType: WebResourceErrorType.webContentProcessTerminated, + ), + ); + }, + ); + }, + ); Future _setCreationParams( CreationParams params, { @@ -190,18 +196,22 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { autoMediaPlaybackPolicy: params.autoMediaPlaybackPolicy, ); - final WeakReference weakRef = - WeakReference(callbacksHandler); webView = webViewProxy.createWebView( configuration, - observeValue: ( - String keyPath, - NSObject object, - Map change, - ) { - final double progress = change[NSKeyValueChangeKey.newValue]! as double; - weakRef.target?.onProgress((progress * 100).round()); - }, + observeValue: withWeakRefenceTo( + callbacksHandler, + (WeakReference weakReference) { + return ( + String keyPath, + NSObject object, + Map change, + ) { + final double progress = + change[NSKeyValueChangeKey.newValue]! as double; + weakReference.target?.onProgress((progress * 100).round()); + }; + }, + ), ); webView.setUIDelegate(uiDelegate); @@ -421,10 +431,6 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { @override Future addJavascriptChannels(Set javascriptChannelNames) async { - final WeakReference weakRef = - WeakReference( - javascriptChannelRegistry, - ); await Future.wait( javascriptChannelNames.where( (String channelName) { @@ -434,15 +440,20 @@ class WebKitWebViewPlatformController extends WebViewPlatformController { (String channelName) { final WKScriptMessageHandler handler = webViewProxy.createScriptMessageHandler( - didReceiveScriptMessage: ( - WKUserContentController userContentController, - WKScriptMessage message, - ) { - weakRef.target?.onJavascriptChannelMessage( - message.name, - message.body!.toString(), - ); - }, + didReceiveScriptMessage: withWeakRefenceTo( + javascriptChannelRegistry, + (WeakReference weakReference) { + return ( + WKUserContentController userContentController, + WKScriptMessage message, + ) { + weakReference.target?.onJavascriptChannelMessage( + message.name, + message.body!.toString(), + ); + }; + }, + ), ); _scriptMessageHandlers[channelName] = handler; diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index c8e18c474e6c..07464d6b360e 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -186,134 +186,6 @@ void main() async { verify(mockWebView.loadRequest(request)); }); - test( - 'navigation delegate callback methods do not cause a circular reference', - () async { - // ignore: unused_local_variable - WebKitWebViewPlatformController? controller = - WebKitWebViewPlatformController( - creationParams: CreationParams( - webSettings: WebSettings( - userAgent: const WebSetting.absent(), - hasNavigationDelegate: false, - hasProgressTracking: false, - )), - callbacksHandler: mockCallbacksHandler, - javascriptChannelRegistry: mockJavascriptChannelRegistry, - webViewProxy: mockWebViewWidgetProxy, - configuration: mockWebViewConfiguration, - ); - - final void Function(WKWebView, String) didStartProvisionalNavigation = - verify(mockWebViewWidgetProxy.createNavigationDelegate( - didFinishNavigation: anyNamed('didFinishNavigation'), - didStartProvisionalNavigation: - captureAnyNamed('didStartProvisionalNavigation'), - decidePolicyForNavigationAction: - anyNamed('decidePolicyForNavigationAction'), - didFailNavigation: anyNamed('didFailNavigation'), - didFailProvisionalNavigation: anyNamed('didFailProvisionalNavigation'), - webViewWebContentProcessDidTerminate: - anyNamed('webViewWebContentProcessDidTerminate'), - )).captured.single as void Function(WKWebView, String); - - controller = null; - await runGarbageCollection(); - - didStartProvisionalNavigation(mockWebView, 'https://google.com'); - verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); - }); - - test('uidelgate callback methods do not cause a circular reference', - () async { - // ignore: unused_local_variable - WebKitWebViewPlatformController? controller = - WebKitWebViewPlatformController( - creationParams: CreationParams( - webSettings: WebSettings( - userAgent: const WebSetting.absent(), - hasNavigationDelegate: false, - hasProgressTracking: false, - )), - callbacksHandler: mockCallbacksHandler, - javascriptChannelRegistry: mockJavascriptChannelRegistry, - webViewProxy: mockWebViewWidgetProxy, - configuration: mockWebViewConfiguration, - ); - - // Lazy initialization. - // ignore: unnecessary_statements - controller.uiDelegate; - - final void Function(WKWebView, String) didStartProvisionalNavigation = - verify(mockWebViewWidgetProxy.createNavigationDelegate( - didFinishNavigation: anyNamed('didFinishNavigation'), - didStartProvisionalNavigation: - captureAnyNamed('didStartProvisionalNavigation'), - decidePolicyForNavigationAction: - anyNamed('decidePolicyForNavigationAction'), - didFailNavigation: anyNamed('didFailNavigation'), - didFailProvisionalNavigation: anyNamed('didFailProvisionalNavigation'), - webViewWebContentProcessDidTerminate: - anyNamed('webViewWebContentProcessDidTerminate'), - )).captured.single as void Function(WKWebView, String); - - controller = null; - await runGarbageCollection(); - - didStartProvisionalNavigation(mockWebView, 'https://google.com'); - verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); - }); - - test( - 'ScriptMessageHandler callback method does not cause a circular reference', - () async { - // ignore: unused_local_variable - WebKitWebViewPlatformController? controller = - WebKitWebViewPlatformController( - creationParams: CreationParams( - webSettings: WebSettings( - userAgent: const WebSetting.absent(), - hasNavigationDelegate: false, - hasProgressTracking: false, - )), - callbacksHandler: mockCallbacksHandler, - javascriptChannelRegistry: mockJavascriptChannelRegistry, - webViewProxy: mockWebViewWidgetProxy, - configuration: mockWebViewConfiguration, - ); - - when( - mockWebViewWidgetProxy.createScriptMessageHandler( - didReceiveScriptMessage: anyNamed('didReceiveScriptMessage'), - ), - ).thenReturn(MockWKScriptMessageHandler()); - - await controller.addJavascriptChannels({'oijw'}); - - final void Function(WKWebView, String) didStartProvisionalNavigation = - verify(mockWebViewWidgetProxy.createNavigationDelegate( - didFinishNavigation: anyNamed('didFinishNavigation'), - didStartProvisionalNavigation: - captureAnyNamed('didStartProvisionalNavigation'), - decidePolicyForNavigationAction: - anyNamed('decidePolicyForNavigationAction'), - didFailNavigation: anyNamed('didFailNavigation'), - didFailProvisionalNavigation: anyNamed('didFailProvisionalNavigation'), - webViewWebContentProcessDidTerminate: - anyNamed('webViewWebContentProcessDidTerminate'), - )).captured.single as void Function(WKWebView, String); - - controller = null; - await runGarbageCollection(); - await runGarbageCollection(); - await runGarbageCollection(); - await runGarbageCollection(); - - didStartProvisionalNavigation(mockWebView, 'https://google.com'); - verifyNever(mockCallbacksHandler.onPageStarted('https://google.com')); - }); - group('CreationParams', () { testWidgets('initialUrl', (WidgetTester tester) async { await buildWidget( From 2af6812ec825b2eb71be56057108b3e5daa02854 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:06:27 -0700 Subject: [PATCH 09/17] docs and remove runGarbageCollection --- .../lib/src/common/weak_reference_utils.dart | 22 +++++++++++++++++ .../webview_flutter_wkwebview/pubspec.yaml | 1 - .../test/src/web_kit_webview_widget_test.dart | 24 ------------------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart index 5e7a871c7792..d26308c83838 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart @@ -1,4 +1,26 @@ /// Helper method for creating callbacks methods with a weak reference. +/// +/// Example: +/// ``` +/// final JavascriptChannelRegistry javascriptChannelRegistry = ... +/// +/// final WKScriptMessageHandler handler = WKScriptMessageHandler( +/// didReceiveScriptMessage: withWeakRefenceTo( +/// javascriptChannelRegistry, +/// (WeakReference weakReference) { +/// return ( +/// WKUserContentController userContentController, +/// WKScriptMessage message, +/// ) { +/// weakReference.target?.onJavascriptChannelMessage( +/// message.name, +/// message.body!.toString(), +/// ); +/// }; +/// }, +/// ), +/// ); +/// ``` S withWeakRefenceTo( T reference, S Function(WeakReference weakReference) onCreate, diff --git a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml index 56150d944213..c69d0d51b622 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml +++ b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml @@ -30,4 +30,3 @@ dev_dependencies: mockito: ^5.1.0 pedantic: ^1.10.0 pigeon: ^3.0.3 - vm_service: ">=8.3.0 <10.0.0" diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index 07464d6b360e..b08224b37619 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -24,22 +24,6 @@ import 'package:webview_flutter_wkwebview/src/web_kit_webview_widget.dart'; import 'web_kit_webview_widget_test.mocks.dart'; -List _cleanupPathSegments(Uri uri) { - final List pathSegments = []; - if (uri.pathSegments.isNotEmpty) { - pathSegments.addAll(uri.pathSegments.where( - (String s) => s.isNotEmpty, - )); - } - return pathSegments; -} - -String _toWebSocket(Uri uri) { - final List pathSegments = _cleanupPathSegments(uri); - pathSegments.add('ws'); - return uri.replace(scheme: 'ws', pathSegments: pathSegments).toString(); -} - @GenerateMocks([ UIScrollView, WKNavigationDelegate, @@ -149,14 +133,6 @@ void main() async { await tester.pumpAndSettle(); } - Future runGarbageCollection() async { - final Uri? serverUri = (await Service.getInfo()).serverUri; - final String isolateId = Service.getIsolateID(Isolate.current)!; - final vm_service.VmService vmService = - await vm_service_io.vmServiceConnectUri(_toWebSocket(serverUri!)); - await vmService.getAllocationProfile(isolateId, gc: true); - } - testWidgets('build $WebKitWebViewWidget', (WidgetTester tester) async { await buildWidget(tester); }); From 1114c07e455475403ac7469078ce6e8e14c51c18 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:37:52 -0700 Subject: [PATCH 10/17] unused imports --- .../test/src/web_kit_webview_widget_test.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index b08224b37619..5c17641a4c9b 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:developer'; -import 'dart:isolate'; import 'dart:math'; // TODO(a14n): remove this import once Flutter 3.1 or later reaches stable (including flutter/flutter#106316) // ignore: unnecessary_import @@ -14,8 +12,6 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; -import 'package:vm_service/vm_service.dart' as vm_service; -import 'package:vm_service/vm_service_io.dart' as vm_service_io; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; import 'package:webview_flutter_wkwebview/src/foundation/foundation.dart'; import 'package:webview_flutter_wkwebview/src/ui_kit/ui_kit.dart'; From c66e8eca03b105da9630388f2a1e1f34b8fa7570 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:41:49 -0700 Subject: [PATCH 11/17] license --- .../lib/src/common/weak_reference_utils.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart index d26308c83838..ad0c9ebf4f5c 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart @@ -1,3 +1,7 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + /// Helper method for creating callbacks methods with a weak reference. /// /// Example: From 75cab298eb1c1459dcd57c8a239b8ee2e862ccf7 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:45:37 -0700 Subject: [PATCH 12/17] remove async from test --- .../test/src/web_kit_webview_widget_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart index 5c17641a4c9b..b5f7b1a486dd 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/web_kit_webview_widget_test.dart @@ -34,7 +34,7 @@ import 'web_kit_webview_widget_test.mocks.dart'; WebViewPlatformCallbacksHandler, WebViewWidgetProxy, ]) -void main() async { +void main() { TestWidgetsFlutterBinding.ensureInitialized(); group('WebKitWebViewWidget', () { From 2f9e913455fc6c35968c9389fbc1a05b85b215cb Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Thu, 30 Jun 2022 11:30:14 -0700 Subject: [PATCH 13/17] add a test --- .../webview_flutter_wkwebview/pubspec.yaml | 1 + .../src/common/instance_manager_test.dart | 76 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml index c69d0d51b622..6bcf822d399a 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml +++ b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml @@ -30,3 +30,4 @@ dev_dependencies: mockito: ^5.1.0 pedantic: ^1.10.0 pigeon: ^3.0.3 + vm_service: ^8.3.0 diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart index 2fc68a489b6a..9c7bdd5578cf 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart @@ -2,8 +2,16 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; +import 'dart:developer'; +import 'dart:isolate'; + +import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:vm_service/vm_service.dart' as vm_service; +import 'package:vm_service/vm_service_io.dart'; import 'package:webview_flutter_wkwebview/src/common/instance_manager.dart'; +import 'package:webview_flutter_wkwebview/src/common/weak_reference_utils.dart'; void main() { group('InstanceManager', () { @@ -132,6 +140,28 @@ void main() { )!; expect(identical(object, newWeakCopy), isFalse); }); + + test('withWeakRefenceTo allows encapsulating class to be garbage collected', + () async { + final Completer gcCompleter = Completer(); + final InstanceManager instanceManager = InstanceManager( + onWeakReferenceRemoved: gcCompleter.complete, + ); + + ClassWithCallbackClass? instance = ClassWithCallbackClass(); + instanceManager.addHostCreatedInstance(instance.callbackClass, 0); + instance = null; + + // Force garbage collection. + final Uri serverUri = (await Service.getInfo()).serverUri!; + + final String isolateId = Service.getIsolateID(Isolate.current)!; + final vm_service.VmService vmService = + await vmServiceConnectUri(_toWebSocket(serverUri)); + await vmService.getAllocationProfile(isolateId, gc: true); + + expect(gcCompleter.future, completion(0)); + }); }); } @@ -151,3 +181,49 @@ class CopyableObject with Copyable { return other is CopyableObject; } } + +class CopyableObjectWithCallback with Copyable { + CopyableObjectWithCallback(this.callback); + + final VoidCallback callback; + + @override + CopyableObjectWithCallback copy() { + return CopyableObjectWithCallback(callback); + } +} + +class ClassWithCallbackClass { + ClassWithCallbackClass() { + callbackClass = CopyableObjectWithCallback( + withWeakRefenceTo( + this, + (WeakReference weakReference) { + return () { + // Weak reference to `this` in callback. + // ignore: unnecessary_statements + weakReference; + }; + }, + ), + ); + } + + late final CopyableObjectWithCallback callbackClass; +} + +List _cleanupPathSegments(Uri uri) { + final List pathSegments = []; + if (uri.pathSegments.isNotEmpty) { + pathSegments.addAll(uri.pathSegments.where( + (String s) => s.isNotEmpty, + )); + } + return pathSegments; +} + +String _toWebSocket(Uri uri) { + final List pathSegments = _cleanupPathSegments(uri); + pathSegments.add('ws'); + return uri.replace(scheme: 'ws', pathSegments: pathSegments).toString(); +} From 7b9261f3a00ec9d42586a43629dd6ec7e86950e0 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Thu, 30 Jun 2022 11:31:55 -0700 Subject: [PATCH 14/17] include weakReferenceTo in doc --- .../lib/src/foundation/foundation.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart index ef08bed31f94..2059aa544207 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/lib/src/foundation/foundation.dart @@ -8,6 +8,7 @@ import 'dart:typed_data'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; +import 'package:webview_flutter_wkwebview/src/common/weak_reference_utils.dart'; import '../common/instance_manager.dart'; import 'foundation_api_impls.dart'; @@ -277,6 +278,8 @@ class NSObject with Copyable { /// `WeakReference` when referencing an object not received as a parameter. /// Otherwise, use [NSObject.dispose] to release the associated Objective-C /// object manually. + /// + /// See [withWeakRefenceTo]. /// {@endtemplate} final void Function( String keyPath, From 1610344015b87307c616d9152419a7076041bdcd Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Thu, 30 Jun 2022 13:00:53 -0700 Subject: [PATCH 15/17] move test to integration test --- .../webview_flutter_test.dart | 56 ++++++++++++++ .../webview_flutter_wkwebview/pubspec.yaml | 1 - .../src/common/instance_manager_test.dart | 76 ------------------- 3 files changed, 56 insertions(+), 77 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart index 9687c7d56cbe..c643c4d6b7d6 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart @@ -22,6 +22,8 @@ import 'package:webview_flutter_platform_interface/webview_flutter_platform_inte import 'package:webview_flutter_wkwebview_example/navigation_decision.dart'; import 'package:webview_flutter_wkwebview_example/navigation_request.dart'; import 'package:webview_flutter_wkwebview_example/web_view.dart'; +import 'package:webview_flutter_wkwebview/src/common/weak_reference_utils.dart'; +import 'package:webview_flutter_wkwebview/src/common/instance_manager.dart'; Future main() async { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); @@ -66,6 +68,30 @@ Future main() async { expect(currentUrl, primaryUrl); }); + testWidgets( + 'withWeakRefenceTo allows encapsulating class to be garbage collected', + (WidgetTester tester) async { + final Completer gcCompleter = Completer(); + final InstanceManager instanceManager = InstanceManager( + onWeakReferenceRemoved: (_) { + print(_); + gcCompleter.complete(_); + }, + ); + + ClassWithCallbackClass? instance = ClassWithCallbackClass(); + instanceManager.addHostCreatedInstance(instance.callbackClass, 0); + instance = null; + + // Force garbage collection. + await IntegrationTestWidgetsFlutterBinding.instance + .watchPerformance(() async { + await tester.pumpAndSettle(); + }); + + expect(gcCompleter.future, completion(0)); + }, timeout: const Timeout(Duration(seconds: 10))); + testWidgets('loadUrl', (WidgetTester tester) async { final Completer controllerCompleter = Completer(); @@ -1253,3 +1279,33 @@ class ResizableWebViewState extends State { ); } } + +class CopyableObjectWithCallback with Copyable { + CopyableObjectWithCallback(this.callback); + + final VoidCallback callback; + + @override + CopyableObjectWithCallback copy() { + return CopyableObjectWithCallback(callback); + } +} + +class ClassWithCallbackClass { + ClassWithCallbackClass() { + callbackClass = CopyableObjectWithCallback( + withWeakRefenceTo( + this, + (WeakReference weakReference) { + return () { + // Weak reference to `this` in callback. + // ignore: unnecessary_statements + weakReference; + }; + }, + ), + ); + } + + late final CopyableObjectWithCallback callbackClass; +} diff --git a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml index 6bcf822d399a..c69d0d51b622 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml +++ b/packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml @@ -30,4 +30,3 @@ dev_dependencies: mockito: ^5.1.0 pedantic: ^1.10.0 pigeon: ^3.0.3 - vm_service: ^8.3.0 diff --git a/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart index 9c7bdd5578cf..2fc68a489b6a 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/test/src/common/instance_manager_test.dart @@ -2,16 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; -import 'dart:developer'; -import 'dart:isolate'; - -import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:vm_service/vm_service.dart' as vm_service; -import 'package:vm_service/vm_service_io.dart'; import 'package:webview_flutter_wkwebview/src/common/instance_manager.dart'; -import 'package:webview_flutter_wkwebview/src/common/weak_reference_utils.dart'; void main() { group('InstanceManager', () { @@ -140,28 +132,6 @@ void main() { )!; expect(identical(object, newWeakCopy), isFalse); }); - - test('withWeakRefenceTo allows encapsulating class to be garbage collected', - () async { - final Completer gcCompleter = Completer(); - final InstanceManager instanceManager = InstanceManager( - onWeakReferenceRemoved: gcCompleter.complete, - ); - - ClassWithCallbackClass? instance = ClassWithCallbackClass(); - instanceManager.addHostCreatedInstance(instance.callbackClass, 0); - instance = null; - - // Force garbage collection. - final Uri serverUri = (await Service.getInfo()).serverUri!; - - final String isolateId = Service.getIsolateID(Isolate.current)!; - final vm_service.VmService vmService = - await vmServiceConnectUri(_toWebSocket(serverUri)); - await vmService.getAllocationProfile(isolateId, gc: true); - - expect(gcCompleter.future, completion(0)); - }); }); } @@ -181,49 +151,3 @@ class CopyableObject with Copyable { return other is CopyableObject; } } - -class CopyableObjectWithCallback with Copyable { - CopyableObjectWithCallback(this.callback); - - final VoidCallback callback; - - @override - CopyableObjectWithCallback copy() { - return CopyableObjectWithCallback(callback); - } -} - -class ClassWithCallbackClass { - ClassWithCallbackClass() { - callbackClass = CopyableObjectWithCallback( - withWeakRefenceTo( - this, - (WeakReference weakReference) { - return () { - // Weak reference to `this` in callback. - // ignore: unnecessary_statements - weakReference; - }; - }, - ), - ); - } - - late final CopyableObjectWithCallback callbackClass; -} - -List _cleanupPathSegments(Uri uri) { - final List pathSegments = []; - if (uri.pathSegments.isNotEmpty) { - pathSegments.addAll(uri.pathSegments.where( - (String s) => s.isNotEmpty, - )); - } - return pathSegments; -} - -String _toWebSocket(Uri uri) { - final List pathSegments = _cleanupPathSegments(uri); - pathSegments.add('ws'); - return uri.replace(scheme: 'ws', pathSegments: pathSegments).toString(); -} From 5f51fbd3c83fc6b6ea7e9f5841405d600934e104 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Thu, 30 Jun 2022 13:01:40 -0700 Subject: [PATCH 16/17] dont print --- .../example/integration_test/webview_flutter_test.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart index c643c4d6b7d6..080285c17677 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart @@ -73,10 +73,7 @@ Future main() async { (WidgetTester tester) async { final Completer gcCompleter = Completer(); final InstanceManager instanceManager = InstanceManager( - onWeakReferenceRemoved: (_) { - print(_); - gcCompleter.complete(_); - }, + onWeakReferenceRemoved: gcCompleter.complete, ); ClassWithCallbackClass? instance = ClassWithCallbackClass(); From bba862503fdd2d095ed8dc7094e8e3572c48e27f Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Thu, 30 Jun 2022 13:09:51 -0700 Subject: [PATCH 17/17] organize imports --- .../example/integration_test/webview_flutter_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart b/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart index 080285c17677..1119f7457bc9 100644 --- a/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart +++ b/packages/webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart @@ -19,11 +19,11 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; +import 'package:webview_flutter_wkwebview/src/common/instance_manager.dart'; +import 'package:webview_flutter_wkwebview/src/common/weak_reference_utils.dart'; import 'package:webview_flutter_wkwebview_example/navigation_decision.dart'; import 'package:webview_flutter_wkwebview_example/navigation_request.dart'; import 'package:webview_flutter_wkwebview_example/web_view.dart'; -import 'package:webview_flutter_wkwebview/src/common/weak_reference_utils.dart'; -import 'package:webview_flutter_wkwebview/src/common/instance_manager.dart'; Future main() async { IntegrationTestWidgetsFlutterBinding.ensureInitialized();