From 105d93ffdc6fe9ed0b3eb170fd5fda4d4fa4e34d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 15 Feb 2022 15:18:48 -0500 Subject: [PATCH] [webview_flutter] Fix debuggingEnabled on Android The recent Dart rewrite accidentally dropped a parameter value in the call chain, causing disabling debugging to enabled it instead. This fixes the issue, and adds testing of that portion of the call chain (which was entirely missing), as well as adding testing of `false` at various other points just for added coverage. As opportunistic cleanup, changes the test group naming, as the pattern of using `'$FooClass'` breaks test result display in VSCode. Fixes https://github.com/flutter/flutter/issues/98521 --- .../webview_flutter_android/CHANGELOG.md | 3 +- .../lib/webview_android_widget.dart | 2 +- .../webview_flutter_android/pubspec.yaml | 2 +- .../test/android_webview_test.dart | 19 +++--- .../test/instance_manager_test.dart | 2 +- .../test/webview_android_widget_test.dart | 63 ++++++++++++++++--- 6 files changed, 72 insertions(+), 19 deletions(-) diff --git a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md index 145bc8fcc068..26fb5ab624f8 100644 --- a/packages/webview_flutter/webview_flutter_android/CHANGELOG.md +++ b/packages/webview_flutter/webview_flutter_android/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 2.8.3 +* Fixes a bug causing `debuggingEnabled` to always be set to true. * Fixes an integration test race condition. ## 2.8.2 diff --git a/packages/webview_flutter/webview_flutter_android/lib/webview_android_widget.dart b/packages/webview_flutter/webview_flutter_android/lib/webview_android_widget.dart index bf85ac97687e..7200aaa4a322 100644 --- a/packages/webview_flutter/webview_flutter_android/lib/webview_android_widget.dart +++ b/packages/webview_flutter/webview_flutter_android/lib/webview_android_widget.dart @@ -708,6 +708,6 @@ class WebViewProxy { /// /// See [android_webview.WebView].setWebContentsDebuggingEnabled. Future setWebContentsDebuggingEnabled(bool enabled) { - return android_webview.WebView.setWebContentsDebuggingEnabled(true); + return android_webview.WebView.setWebContentsDebuggingEnabled(enabled); } } diff --git a/packages/webview_flutter/webview_flutter_android/pubspec.yaml b/packages/webview_flutter/webview_flutter_android/pubspec.yaml index b2329d3fb904..704dc6719a7e 100644 --- a/packages/webview_flutter/webview_flutter_android/pubspec.yaml +++ b/packages/webview_flutter/webview_flutter_android/pubspec.yaml @@ -2,7 +2,7 @@ name: webview_flutter_android description: A Flutter plugin that provides a WebView widget on Android. repository: https://github.com/flutter/plugins/tree/main/packages/webview_flutter/webview_flutter_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22 -version: 2.8.2 +version: 2.8.3 environment: sdk: ">=2.14.0 <3.0.0" diff --git a/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart b/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart index 8688a1977d83..91385ff2d364 100644 --- a/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/android_webview_test.dart @@ -32,7 +32,7 @@ void main() { TestWidgetsFlutterBinding.ensureInitialized(); group('Android WebView', () { - group('$WebView', () { + group('WebView', () { late MockTestWebViewHostApi mockPlatformHostApi; late InstanceManager instanceManager; @@ -55,11 +55,16 @@ void main() { verify(mockPlatformHostApi.create(webViewInstanceId, false)); }); - test('setWebContentsDebuggingEnabled', () { + test('setWebContentsDebuggingEnabled true', () { WebView.setWebContentsDebuggingEnabled(true); verify(mockPlatformHostApi.setWebContentsDebuggingEnabled(true)); }); + test('setWebContentsDebuggingEnabled false', () { + WebView.setWebContentsDebuggingEnabled(false); + verify(mockPlatformHostApi.setWebContentsDebuggingEnabled(false)); + }); + test('loadData', () { webView.loadData( data: 'hello', @@ -314,7 +319,7 @@ void main() { }); }); - group('$WebSettings', () { + group('WebSettings', () { late MockTestWebSettingsHostApi mockPlatformHostApi; late InstanceManager instanceManager; @@ -440,7 +445,7 @@ void main() { }); }); - group('$JavaScriptChannel', () { + group('JavaScriptChannel', () { late JavaScriptChannelFlutterApiImpl flutterApi; late InstanceManager instanceManager; @@ -468,7 +473,7 @@ void main() { }); }); - group('$WebViewClient', () { + group('WebViewClient', () { late WebViewClientFlutterApiImpl flutterApi; late InstanceManager instanceManager; @@ -583,7 +588,7 @@ void main() { }); }); - group('$DownloadListener', () { + group('DownloadListener', () { late DownloadListenerFlutterApiImpl flutterApi; late InstanceManager instanceManager; @@ -621,7 +626,7 @@ void main() { }); }); - group('$WebChromeClient', () { + group('WebChromeClient', () { late WebChromeClientFlutterApiImpl flutterApi; late InstanceManager instanceManager; diff --git a/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart b/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart index fd020fc362c8..3aeb005efb86 100644 --- a/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/instance_manager_test.dart @@ -6,7 +6,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:webview_flutter_android/src/instance_manager.dart'; void main() { - group('$InstanceManager', () { + group('InstanceManager', () { late InstanceManager testInstanceManager; setUp(() { diff --git a/packages/webview_flutter/webview_flutter_android/test/webview_android_widget_test.dart b/packages/webview_flutter/webview_flutter_android/test/webview_android_widget_test.dart index fed1c1113e55..af1093914047 100644 --- a/packages/webview_flutter/webview_flutter_android/test/webview_android_widget_test.dart +++ b/packages/webview_flutter/webview_flutter_android/test/webview_android_widget_test.dart @@ -11,9 +11,13 @@ import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; import 'package:webview_flutter_android/src/android_webview.dart' as android_webview; +import 'package:webview_flutter_android/src/android_webview_api_impls.dart'; +import 'package:webview_flutter_android/src/instance_manager.dart'; import 'package:webview_flutter_android/webview_android_widget.dart'; import 'package:webview_flutter_platform_interface/webview_flutter_platform_interface.dart'; +import 'android_webview.pigeon.dart'; +import 'android_webview_test.mocks.dart' show MockTestWebViewHostApi; import 'webview_android_widget_test.mocks.dart'; @GenerateMocks([ @@ -31,7 +35,7 @@ import 'webview_android_widget_test.mocks.dart'; void main() { TestWidgetsFlutterBinding.ensureInitialized(); - group('$WebViewAndroidWidget', () { + group('WebViewAndroidWidget', () { late MockFlutterAssetManager mockFlutterAssetManager; late MockWebView mockWebView; late MockWebSettings mockWebSettings; @@ -93,7 +97,7 @@ void main() { webChromeClient = testController.webChromeClient; } - testWidgets('$WebViewAndroidWidget', (WidgetTester tester) async { + testWidgets('WebViewAndroidWidget', (WidgetTester tester) async { await buildWidget(tester); verify(mockWebSettings.setDomStorageEnabled(true)); @@ -119,7 +123,7 @@ void main() { }, ); - group('$CreationParams', () { + group('CreationParams', () { testWidgets('initialUrl', (WidgetTester tester) async { await buildWidget( tester, @@ -201,7 +205,7 @@ void main() { expect(javaScriptChannels[1].channelName, 'b'); }); - group('$WebSettings', () { + group('WebSettings', () { testWidgets('javascriptMode', (WidgetTester tester) async { await buildWidget( tester, @@ -232,7 +236,7 @@ void main() { expect(testController.webViewClient.shouldOverrideUrlLoading, isTrue); }); - testWidgets('debuggingEnabled', (WidgetTester tester) async { + testWidgets('debuggingEnabled true', (WidgetTester tester) async { await buildWidget( tester, creationParams: CreationParams( @@ -247,6 +251,21 @@ void main() { verify(mockWebViewProxy.setWebContentsDebuggingEnabled(true)); }); + testWidgets('debuggingEnabled false', (WidgetTester tester) async { + await buildWidget( + tester, + creationParams: CreationParams( + webSettings: WebSettings( + userAgent: const WebSetting.absent(), + debuggingEnabled: false, + hasNavigationDelegate: false, + ), + ), + ); + + verify(mockWebViewProxy.setWebContentsDebuggingEnabled(false)); + }); + testWidgets('userAgent', (WidgetTester tester) async { await buildWidget( tester, @@ -278,7 +297,7 @@ void main() { }); }); - group('$WebViewPlatformController', () { + group('WebViewPlatformController', () { testWidgets('loadFile without "file://" prefix', (WidgetTester tester) async { await buildWidget(tester); @@ -667,7 +686,7 @@ void main() { }); }); - group('$WebViewPlatformCallbacksHandler', () { + group('WebViewPlatformCallbacksHandler', () { testWidgets('onPageStarted', (WidgetTester tester) async { await buildWidget(tester); webViewClient.onPageStarted(mockWebView, 'https://google.com'); @@ -773,7 +792,7 @@ void main() { verify(mockWebView.loadUrl('https://google.com', {})); }); - group('$JavascriptChannelRegistry', () { + group('JavascriptChannelRegistry', () { testWidgets('onJavascriptChannelMessage', (WidgetTester tester) async { await buildWidget(tester); @@ -792,4 +811,32 @@ void main() { }); }); }); + + group('WebViewProxy', () { + late MockTestWebViewHostApi mockPlatformHostApi; + late InstanceManager instanceManager; + + setUp(() { + // WebViewProxy calls static methods that can't be mocked, so the mocks + // have to be set up at the next layer down, by mocking the implementation + // of WebView itstelf. + mockPlatformHostApi = MockTestWebViewHostApi(); + TestWebViewHostApi.setup(mockPlatformHostApi); + instanceManager = InstanceManager(); + android_webview.WebView.api = + WebViewHostApiImpl(instanceManager: instanceManager); + }); + + test('setWebContentsDebuggingEnabled true', () { + const WebViewProxy webViewProxy = WebViewProxy(); + webViewProxy.setWebContentsDebuggingEnabled(true); + verify(mockPlatformHostApi.setWebContentsDebuggingEnabled(true)); + }); + + test('setWebContentsDebuggingEnabled false', () { + const WebViewProxy webViewProxy = WebViewProxy(); + webViewProxy.setWebContentsDebuggingEnabled(false); + verify(mockPlatformHostApi.setWebContentsDebuggingEnabled(false)); + }); + }); }