From f7cdea8ea99b5186bbf02659b4b6a64f783c6d6b Mon Sep 17 00:00:00 2001 From: Amir Hardon Date: Tue, 21 May 2019 17:21:36 -0700 Subject: [PATCH 1/5] platform_interface: Use Dart objects for creationParams and webSettings. As a temporary step we passed these objects as maps that were sent across the method channel. This PR replaces the maps with proper Dart objects. --- .../lib/platform_interface.dart | 74 +++++++++- .../lib/src/webview_android.dart | 4 +- .../lib/src/webview_cupertino.dart | 4 +- .../lib/src/webview_method_channel.dart | 36 +++++ .../webview_flutter/lib/webview_flutter.dart | 130 ++++++------------ .../test/webview_flutter_test.dart | 59 ++++++-- 6 files changed, 205 insertions(+), 102 deletions(-) diff --git a/packages/webview_flutter/lib/platform_interface.dart b/packages/webview_flutter/lib/platform_interface.dart index f5918d80aef6..2ea530d56a76 100644 --- a/packages/webview_flutter/lib/platform_interface.dart +++ b/packages/webview_flutter/lib/platform_interface.dart @@ -8,6 +8,8 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/widgets.dart'; +import 'webview_flutter.dart'; + /// Interface for talking to the webview's platform implementation. /// /// An instance implementing this interface is passed to the `onWebViewPlatformCreated` callback that is @@ -29,6 +31,15 @@ abstract class WebViewPlatform { "WebView loadUrl is not implemented on the current platform"); } + /// Updates the webview settings. + /// + /// Any non null field in `settings` will be set as the new setting value. + /// All null fields in `settings` are ignored. + Future updateSettings(WebSettings setting) { + throw UnimplementedError( + "WebView updateSettings is not implemented on the current platform"); + } + // As the PR currently focus about the wiring I've only moved loadUrl to the new way, so // the discussion is more focused. // In this temporary state WebViewController still uses a method channel directly for all other @@ -37,6 +48,67 @@ abstract class WebViewPlatform { int get id; } +/// Settings for configuring a WebViewPlatform. +/// +/// Initial settings are passed as part of [CreationParams], settings updates are sent with +/// [WebViewPlatform#updateSettings]. +class WebSettings { + WebSettings({ + this.javascriptMode, + this.hasNavigationDelegate, + this.debuggingEnabled, + }); + + final JavascriptMode javascriptMode; + + /// Whether a [NavigationDelegate] should be used for this webview. + final bool hasNavigationDelegate; + + /// Whether to enable the platform's webview content debugging tools. + /// + /// See also: [WebView.debuggingEnabled]. + final bool debuggingEnabled; + + @override + String toString() { + return 'WebSettings(javascriptMode: $javascriptMode, hasNavigationDelegate: $hasNavigationDelegate, debuggingEnabled: $debuggingEnabled)'; + } +} + +/// Configuration to use when creating a new [WebViewPlatform]. +class CreationParams { + CreationParams( + {this.initialUrl, this.webSettings, this.javascriptChannelNames}); + + /// The initialUrl to load in the webview. + /// + /// When null the webview will be created without loading any page. + final String initialUrl; + + /// The initial [WebSettings] for the new webview. + /// + /// This can later be updated with [WebViewPlatform.updateSettings]. + final WebSettings webSettings; + + /// The initial set of JavaScript channels that are configured for this webview. + /// + /// For each value in this list the platform's webview should make sure that a corresponding + /// property with a postMessage method is set on `window`. For example for a JavaScript channel + /// named `Foo` it should be possible for JavaScript code executing in the webview to do + /// + /// ```javascript + /// Foo.postMessage('hello'); + /// ``` + // TODO(amirh): describe what should happen when postMessage is called once that code is migrated + // to PlatformWebView. + final Set javascriptChannelNames; + + @override + String toString() { + return '$runtimeType(initialUrl: $initialUrl, settings: $webSettings, javascriptChannelNames: $javascriptChannelNames)'; + } +} + typedef WebViewPlatformCreatedCallback = void Function( WebViewPlatform webViewPlatform); @@ -67,7 +139,7 @@ abstract class WebViewBuilder { // TODO(amirh): convert this to be the actual parameters. // I'm starting without it as the PR is starting to become pretty big. // I'll followup with the conversion PR. - Map creationParams, + CreationParams creationParams, WebViewPlatformCreatedCallback onWebViewPlatformCreated, Set> gestureRecognizers, }); diff --git a/packages/webview_flutter/lib/src/webview_android.dart b/packages/webview_flutter/lib/src/webview_android.dart index 86a20b27dcb2..166829f773da 100644 --- a/packages/webview_flutter/lib/src/webview_android.dart +++ b/packages/webview_flutter/lib/src/webview_android.dart @@ -19,7 +19,7 @@ class AndroidWebViewBuilder implements WebViewBuilder { @override Widget build({ BuildContext context, - Map creationParams, + CreationParams creationParams, WebViewPlatformCreatedCallback onWebViewPlatformCreated, Set> gestureRecognizers, }) { @@ -46,7 +46,7 @@ class AndroidWebViewBuilder implements WebViewBuilder { // we explicitly set it here so that the widget doesn't require an ambient // directionality. layoutDirection: TextDirection.rtl, - creationParams: creationParams, + creationParams: MethodChannelWebViewPlatform.creationParamsToMap(creationParams), creationParamsCodec: const StandardMessageCodec(), ), ); diff --git a/packages/webview_flutter/lib/src/webview_cupertino.dart b/packages/webview_flutter/lib/src/webview_cupertino.dart index e5487f435230..535bbc2471ca 100644 --- a/packages/webview_flutter/lib/src/webview_cupertino.dart +++ b/packages/webview_flutter/lib/src/webview_cupertino.dart @@ -19,7 +19,7 @@ class CupertinoWebViewBuilder implements WebViewBuilder { @override Widget build({ BuildContext context, - Map creationParams, + CreationParams creationParams, WebViewPlatformCreatedCallback onWebViewPlatformCreated, Set> gestureRecognizers, }) { @@ -32,7 +32,7 @@ class CupertinoWebViewBuilder implements WebViewBuilder { onWebViewPlatformCreated(MethodChannelWebViewPlatform(id)); }, gestureRecognizers: gestureRecognizers, - creationParams: creationParams, + creationParams: MethodChannelWebViewPlatform.creationParamsToMap(creationParams), creationParamsCodec: const StandardMessageCodec(), ); } diff --git a/packages/webview_flutter/lib/src/webview_method_channel.dart b/packages/webview_flutter/lib/src/webview_method_channel.dart index e13fe6f4e5e5..a07dc7163f39 100644 --- a/packages/webview_flutter/lib/src/webview_method_channel.dart +++ b/packages/webview_flutter/lib/src/webview_method_channel.dart @@ -32,6 +32,42 @@ class MethodChannelWebViewPlatform implements WebViewPlatform { }); } + @override + Future updateSettings(WebSettings settings) { + // TODO(amirh): remove this on when the invokeMethod update makes it to stable Flutter. + // https://github.com/flutter/flutter/issues/26431 + // ignore: strong_mode_implicit_dynamic_method + final Map updatesMap = _webSettingsToMap(settings); + if (updatesMap.isEmpty) { + return null; + } + return _channel.invokeMethod('updateSettings', updatesMap); + } + + static Map _webSettingsToMap(WebSettings settings) { + final Map map = {}; + void _addIfNonNull(String key, dynamic value) { + if (value == null) { + return; + } + map[key] = value; + } + + _addIfNonNull('jsMode', settings.javascriptMode?.index); + _addIfNonNull('hasNavigationDelegate', settings.hasNavigationDelegate); + _addIfNonNull('debuggingEnabled', settings.debuggingEnabled); + return map; + } + + + static Map creationParamsToMap(CreationParams creationParams) { + return { + 'initialUrl': creationParams.initialUrl, + 'settings': _webSettingsToMap(creationParams.webSettings), + 'javascriptChannelNames': creationParams.javascriptChannelNames.toList(), + }; + } + @override int get id => _id; } diff --git a/packages/webview_flutter/lib/webview_flutter.dart b/packages/webview_flutter/lib/webview_flutter.dart index 432de6e95d5a..b38aaac7b702 100644 --- a/packages/webview_flutter/lib/webview_flutter.dart +++ b/packages/webview_flutter/lib/webview_flutter.dart @@ -268,9 +268,9 @@ class _WebViewState extends State { Widget build(BuildContext context) { return WebView.platformBuilder.build( context: context, - creationParams: _CreationParams.fromWidget(widget).toMap(), onWebViewPlatformCreated: _onWebViewPlatformCreated, gestureRecognizers: widget.gestureRecognizers, + creationParams: _creationParamsfromWidget(widget), ); } @@ -307,85 +307,51 @@ class _WebViewState extends State { } } -Set _extractChannelNames(Set channels) { - final Set channelNames = channels == null - // TODO(iskakaushik): Remove this when collection literals makes it to stable. - // ignore: prefer_collection_literals - ? Set() - : channels.map((JavascriptChannel channel) => channel.name).toSet(); - return channelNames; +CreationParams _creationParamsfromWidget(WebView widget) { + return CreationParams( + initialUrl: widget.initialUrl, + webSettings: _webSettingsFromWidget(widget), + javascriptChannelNames: + _extractChannelNames(widget.javascriptChannels), + ); } -class _CreationParams { - _CreationParams( - {this.initialUrl, this.settings, this.javascriptChannelNames}); - - static _CreationParams fromWidget(WebView widget) { - return _CreationParams( - initialUrl: widget.initialUrl, - settings: _WebSettings.fromWidget(widget), - javascriptChannelNames: - _extractChannelNames(widget.javascriptChannels).toList(), - ); - } - - final String initialUrl; - - final _WebSettings settings; - - final List javascriptChannelNames; - - Map toMap() { - return { - 'initialUrl': initialUrl, - 'settings': settings.toMap(), - 'javascriptChannelNames': javascriptChannelNames, - }; - } +WebSettings _webSettingsFromWidget(WebView widget) { + return WebSettings( + javascriptMode: widget.javascriptMode, + hasNavigationDelegate: widget.navigationDelegate != null, + debuggingEnabled: widget.debuggingEnabled, + ); } -class _WebSettings { - _WebSettings({ - this.javascriptMode, - this.hasNavigationDelegate, - this.debuggingEnabled, - }); - - static _WebSettings fromWidget(WebView widget) { - return _WebSettings( - javascriptMode: widget.javascriptMode, - hasNavigationDelegate: widget.navigationDelegate != null, - debuggingEnabled: widget.debuggingEnabled, - ); +WebSettings _webSettingsUpdate(WebSettings currentValue, WebSettings newValue) { + JavascriptMode javascriptMode; + bool hasNavigationDelegate; + bool debuggingEnabled; + if (currentValue.javascriptMode != newValue.javascriptMode) { + javascriptMode = newValue.javascriptMode; } - - final JavascriptMode javascriptMode; - final bool hasNavigationDelegate; - final bool debuggingEnabled; - - Map toMap() { - return { - 'jsMode': javascriptMode.index, - 'hasNavigationDelegate': hasNavigationDelegate, - 'debuggingEnabled': debuggingEnabled, - }; + if (currentValue.hasNavigationDelegate != newValue.hasNavigationDelegate) { + hasNavigationDelegate = newValue.hasNavigationDelegate; + } + if (currentValue.debuggingEnabled != newValue.debuggingEnabled) { + debuggingEnabled = newValue.debuggingEnabled; } - Map updatesMap(_WebSettings newSettings) { - final Map updates = {}; - if (javascriptMode != newSettings.javascriptMode) { - updates['jsMode'] = newSettings.javascriptMode.index; - } - if (hasNavigationDelegate != newSettings.hasNavigationDelegate) { - updates['hasNavigationDelegate'] = newSettings.hasNavigationDelegate; - } - - if (debuggingEnabled != newSettings.debuggingEnabled) { - updates['debuggingEnabled'] = newSettings.debuggingEnabled; - } + return WebSettings( + javascriptMode: javascriptMode, + hasNavigationDelegate: hasNavigationDelegate, + debuggingEnabled: debuggingEnabled + ); +} - return updates; - } +Set _extractChannelNames(Set channels) { + final Set channelNames = channels == null + // TODO(iskakaushik): Remove this when collection literals makes it to stable. + // ignore: prefer_collection_literals + ? Set() + : channels.map((JavascriptChannel channel) => channel.name).toSet(); + return channelNames; } /// Controls a [WebView]. @@ -398,7 +364,7 @@ class WebViewController { this._platformInterface, this._widget, ) : _channel = MethodChannel('plugins.flutter.io/webview_$id') { - _settings = _WebSettings.fromWidget(_widget); + _settings = _webSettingsFromWidget(_widget); _updateJavascriptChannelsFromSet(_widget.javascriptChannels); _channel.setMethodCallHandler(_onMethodCall); } @@ -407,7 +373,7 @@ class WebViewController { final WebViewPlatform _platformInterface; - _WebSettings _settings; + WebSettings _settings; WebView _widget; @@ -549,20 +515,14 @@ class WebViewController { Future _updateWidget(WebView widget) async { _widget = widget; - await _updateSettings(_WebSettings.fromWidget(widget)); + await _updateSettings(_webSettingsFromWidget(widget)); await _updateJavascriptChannels(widget.javascriptChannels); } - Future _updateSettings(_WebSettings setting) async { - final Map updateMap = _settings.updatesMap(setting); - if (updateMap == null || updateMap.isEmpty) { - return null; - } - _settings = setting; - // TODO(amirh): remove this on when the invokeMethod update makes it to stable Flutter. - // https://github.com/flutter/flutter/issues/26431 - // ignore: strong_mode_implicit_dynamic_method - return _channel.invokeMethod('updateSettings', updateMap); + Future _updateSettings(WebSettings newSettings) { + final WebSettings update = _webSettingsUpdate(_settings, newSettings); + _settings = newSettings; + return _platformInterface.updateSettings(update); } Future _updateJavascriptChannels( diff --git a/packages/webview_flutter/test/webview_flutter_test.dart b/packages/webview_flutter/test/webview_flutter_test.dart index 3b34b858078b..329bfa85437a 100644 --- a/packages/webview_flutter/test/webview_flutter_test.dart +++ b/packages/webview_flutter/test/webview_flutter_test.dart @@ -66,7 +66,6 @@ void main() { initialUrl: 'https://youtube.com', javascriptMode: JavascriptMode.disabled, )); - expect(platformWebView.javascriptMode, JavascriptMode.disabled); }); @@ -769,15 +768,18 @@ void main() { final MyWebViewBuilder builder = WebView.platformBuilder; final MyWebViewPlatform platform = builder.lastPlatformBuilt; - expect(platform.creationParams, { - 'initialUrl': 'https://youtube.com', - 'settings': { - 'jsMode': 0, - 'hasNavigationDelegate': false, - 'debuggingEnabled': false - }, - 'javascriptChannelNames': [], - }); + expect(platform.creationParams, + MatchesCreationParams(CreationParams( + initialUrl: 'https://youtube.com', + webSettings: WebSettings( + javascriptMode: JavascriptMode.disabled, + hasNavigationDelegate: false, + debuggingEnabled: false, + ), + // TODO(iskakaushik): Remove this when collection literals makes it to stable. + // ignore: prefer_collection_literals + javascriptChannelNames: Set(), + ))); }); testWidgets('loadUrl', (WidgetTester tester) async { @@ -1036,7 +1038,7 @@ class MyWebViewBuilder implements WebViewBuilder { @override Widget build({ BuildContext context, - Map creationParams, + CreationParams creationParams, @required WebViewPlatformCreatedCallback onWebViewPlatformCreated, Set> gestureRecognizers, }) { @@ -1050,7 +1052,7 @@ class MyWebViewBuilder implements WebViewBuilder { class MyWebViewPlatform extends WebViewPlatform { MyWebViewPlatform(this.creationParams, this.gestureRecognizers); - Map creationParams; + CreationParams creationParams; Set> gestureRecognizers; String lastUrlLoaded; @@ -1058,6 +1060,7 @@ class MyWebViewPlatform extends WebViewPlatform { @override Future loadUrl(String url, Map headers) { + equals(1, 1); lastUrlLoaded = url; lastRequestHeaders = headers; return null; @@ -1067,3 +1070,35 @@ class MyWebViewPlatform extends WebViewPlatform { // TODO: implement id int get id => 1; } + +class MatchesWebSettings extends Matcher { + MatchesWebSettings(this._webSettings); + + final WebSettings _webSettings; + + @override + Description describe(Description description) => description.add('$_webSettings'); + + @override + bool matches(covariant WebSettings webSettings, Map matchState) { + return _webSettings.javascriptMode == webSettings.javascriptMode + && _webSettings.hasNavigationDelegate == webSettings.hasNavigationDelegate + && _webSettings.debuggingEnabled == webSettings.debuggingEnabled; + } +} + +class MatchesCreationParams extends Matcher { + MatchesCreationParams(this._creationParams); + + final CreationParams _creationParams; + + @override + Description describe(Description description) => description.add('$_creationParams'); + + @override + bool matches(covariant CreationParams creationParams, Map matchState) { + return _creationParams.initialUrl == creationParams.initialUrl + && MatchesWebSettings(_creationParams.webSettings).matches(creationParams.webSettings, matchState) + && orderedEquals(_creationParams.javascriptChannelNames).matches(creationParams.javascriptChannelNames, matchState); + } +} From 1b8e20fc646072b39f1b1a01958dd03658362813 Mon Sep 17 00:00:00 2001 From: Amir Hardon Date: Wed, 22 May 2019 14:44:24 -0700 Subject: [PATCH 2/5] review comments followup --- packages/webview_flutter/lib/platform_interface.dart | 5 +++-- packages/webview_flutter/lib/src/webview_method_channel.dart | 4 ++++ packages/webview_flutter/lib/webview_flutter.dart | 4 ++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/webview_flutter/lib/platform_interface.dart b/packages/webview_flutter/lib/platform_interface.dart index 2ea530d56a76..df4e3f7d7cca 100644 --- a/packages/webview_flutter/lib/platform_interface.dart +++ b/packages/webview_flutter/lib/platform_interface.dart @@ -59,9 +59,10 @@ class WebSettings { this.debuggingEnabled, }); + /// The JavaScript execution mode to be used by the webview. final JavascriptMode javascriptMode; - /// Whether a [NavigationDelegate] should be used for this webview. + /// Whether the [WebView] has a [NavigationDelegate] set. final bool hasNavigationDelegate; /// Whether to enable the platform's webview content debugging tools. @@ -92,7 +93,7 @@ class CreationParams { /// The initial set of JavaScript channels that are configured for this webview. /// - /// For each value in this list the platform's webview should make sure that a corresponding + /// For each value in this set the platform's webview should make sure that a corresponding /// property with a postMessage method is set on `window`. For example for a JavaScript channel /// named `Foo` it should be possible for JavaScript code executing in the webview to do /// diff --git a/packages/webview_flutter/lib/src/webview_method_channel.dart b/packages/webview_flutter/lib/src/webview_method_channel.dart index a07dc7163f39..2a035a4a8fa3 100644 --- a/packages/webview_flutter/lib/src/webview_method_channel.dart +++ b/packages/webview_flutter/lib/src/webview_method_channel.dart @@ -60,6 +60,10 @@ class MethodChannelWebViewPlatform implements WebViewPlatform { } + /// Converts a [CreationParams] object to a map as expected by `platform_views` channel. + /// + /// This is used for the `creationParams` argument of the platform views created by + /// [AndroidWebViewBuilder] and [CupertinoWebViewBuilder]. static Map creationParamsToMap(CreationParams creationParams) { return { 'initialUrl': creationParams.initialUrl, diff --git a/packages/webview_flutter/lib/webview_flutter.dart b/packages/webview_flutter/lib/webview_flutter.dart index b38aaac7b702..a27e9c578612 100644 --- a/packages/webview_flutter/lib/webview_flutter.dart +++ b/packages/webview_flutter/lib/webview_flutter.dart @@ -324,7 +324,11 @@ WebSettings _webSettingsFromWidget(WebView widget) { ); } +// This method assumes that no fields in `currentValue` are null. WebSettings _webSettingsUpdate(WebSettings currentValue, WebSettings newValue) { + assert(currentValue.javascriptMode != null); + assert(currentValue.hasNavigationDelegate != null); + assert(currentValue.debuggingEnabled != null); JavascriptMode javascriptMode; bool hasNavigationDelegate; bool debuggingEnabled; From f18e066d931681512cae05357f4f5fe6cfa81907 Mon Sep 17 00:00:00 2001 From: Amir Hardon Date: Wed, 22 May 2019 14:47:33 -0700 Subject: [PATCH 3/5] format --- .../lib/src/webview_android.dart | 3 +- .../lib/src/webview_cupertino.dart | 3 +- .../lib/src/webview_method_channel.dart | 6 +-- .../webview_flutter/lib/webview_flutter.dart | 10 ++--- .../test/webview_flutter_test.dart | 38 +++++++++++-------- 5 files changed, 34 insertions(+), 26 deletions(-) diff --git a/packages/webview_flutter/lib/src/webview_android.dart b/packages/webview_flutter/lib/src/webview_android.dart index 166829f773da..7304e9bce22c 100644 --- a/packages/webview_flutter/lib/src/webview_android.dart +++ b/packages/webview_flutter/lib/src/webview_android.dart @@ -46,7 +46,8 @@ class AndroidWebViewBuilder implements WebViewBuilder { // we explicitly set it here so that the widget doesn't require an ambient // directionality. layoutDirection: TextDirection.rtl, - creationParams: MethodChannelWebViewPlatform.creationParamsToMap(creationParams), + creationParams: + MethodChannelWebViewPlatform.creationParamsToMap(creationParams), creationParamsCodec: const StandardMessageCodec(), ), ); diff --git a/packages/webview_flutter/lib/src/webview_cupertino.dart b/packages/webview_flutter/lib/src/webview_cupertino.dart index 535bbc2471ca..aba0a8d264d5 100644 --- a/packages/webview_flutter/lib/src/webview_cupertino.dart +++ b/packages/webview_flutter/lib/src/webview_cupertino.dart @@ -32,7 +32,8 @@ class CupertinoWebViewBuilder implements WebViewBuilder { onWebViewPlatformCreated(MethodChannelWebViewPlatform(id)); }, gestureRecognizers: gestureRecognizers, - creationParams: MethodChannelWebViewPlatform.creationParamsToMap(creationParams), + creationParams: + MethodChannelWebViewPlatform.creationParamsToMap(creationParams), creationParamsCodec: const StandardMessageCodec(), ); } diff --git a/packages/webview_flutter/lib/src/webview_method_channel.dart b/packages/webview_flutter/lib/src/webview_method_channel.dart index 2a035a4a8fa3..4166f9078399 100644 --- a/packages/webview_flutter/lib/src/webview_method_channel.dart +++ b/packages/webview_flutter/lib/src/webview_method_channel.dart @@ -37,7 +37,7 @@ class MethodChannelWebViewPlatform implements WebViewPlatform { // TODO(amirh): remove this on when the invokeMethod update makes it to stable Flutter. // https://github.com/flutter/flutter/issues/26431 // ignore: strong_mode_implicit_dynamic_method - final Map updatesMap = _webSettingsToMap(settings); + final Map updatesMap = _webSettingsToMap(settings); if (updatesMap.isEmpty) { return null; } @@ -59,12 +59,12 @@ class MethodChannelWebViewPlatform implements WebViewPlatform { return map; } - /// Converts a [CreationParams] object to a map as expected by `platform_views` channel. /// /// This is used for the `creationParams` argument of the platform views created by /// [AndroidWebViewBuilder] and [CupertinoWebViewBuilder]. - static Map creationParamsToMap(CreationParams creationParams) { + static Map creationParamsToMap( + CreationParams creationParams) { return { 'initialUrl': creationParams.initialUrl, 'settings': _webSettingsToMap(creationParams.webSettings), diff --git a/packages/webview_flutter/lib/webview_flutter.dart b/packages/webview_flutter/lib/webview_flutter.dart index a27e9c578612..53fa9cf68296 100644 --- a/packages/webview_flutter/lib/webview_flutter.dart +++ b/packages/webview_flutter/lib/webview_flutter.dart @@ -311,8 +311,7 @@ CreationParams _creationParamsfromWidget(WebView widget) { return CreationParams( initialUrl: widget.initialUrl, webSettings: _webSettingsFromWidget(widget), - javascriptChannelNames: - _extractChannelNames(widget.javascriptChannels), + javascriptChannelNames: _extractChannelNames(widget.javascriptChannels), ); } @@ -343,10 +342,9 @@ WebSettings _webSettingsUpdate(WebSettings currentValue, WebSettings newValue) { } return WebSettings( - javascriptMode: javascriptMode, - hasNavigationDelegate: hasNavigationDelegate, - debuggingEnabled: debuggingEnabled - ); + javascriptMode: javascriptMode, + hasNavigationDelegate: hasNavigationDelegate, + debuggingEnabled: debuggingEnabled); } Set _extractChannelNames(Set channels) { diff --git a/packages/webview_flutter/test/webview_flutter_test.dart b/packages/webview_flutter/test/webview_flutter_test.dart index 329bfa85437a..7176756bb79b 100644 --- a/packages/webview_flutter/test/webview_flutter_test.dart +++ b/packages/webview_flutter/test/webview_flutter_test.dart @@ -768,12 +768,13 @@ void main() { final MyWebViewBuilder builder = WebView.platformBuilder; final MyWebViewPlatform platform = builder.lastPlatformBuilt; - expect(platform.creationParams, + expect( + platform.creationParams, MatchesCreationParams(CreationParams( initialUrl: 'https://youtube.com', webSettings: WebSettings( - javascriptMode: JavascriptMode.disabled, - hasNavigationDelegate: false, + javascriptMode: JavascriptMode.disabled, + hasNavigationDelegate: false, debuggingEnabled: false, ), // TODO(iskakaushik): Remove this when collection literals makes it to stable. @@ -1073,17 +1074,20 @@ class MyWebViewPlatform extends WebViewPlatform { class MatchesWebSettings extends Matcher { MatchesWebSettings(this._webSettings); - + final WebSettings _webSettings; - + @override - Description describe(Description description) => description.add('$_webSettings'); + Description describe(Description description) => + description.add('$_webSettings'); @override - bool matches(covariant WebSettings webSettings, Map matchState) { - return _webSettings.javascriptMode == webSettings.javascriptMode - && _webSettings.hasNavigationDelegate == webSettings.hasNavigationDelegate - && _webSettings.debuggingEnabled == webSettings.debuggingEnabled; + bool matches( + covariant WebSettings webSettings, Map matchState) { + return _webSettings.javascriptMode == webSettings.javascriptMode && + _webSettings.hasNavigationDelegate == + webSettings.hasNavigationDelegate && + _webSettings.debuggingEnabled == webSettings.debuggingEnabled; } } @@ -1093,12 +1097,16 @@ class MatchesCreationParams extends Matcher { final CreationParams _creationParams; @override - Description describe(Description description) => description.add('$_creationParams'); + Description describe(Description description) => + description.add('$_creationParams'); @override - bool matches(covariant CreationParams creationParams, Map matchState) { - return _creationParams.initialUrl == creationParams.initialUrl - && MatchesWebSettings(_creationParams.webSettings).matches(creationParams.webSettings, matchState) - && orderedEquals(_creationParams.javascriptChannelNames).matches(creationParams.javascriptChannelNames, matchState); + bool matches(covariant CreationParams creationParams, + Map matchState) { + return _creationParams.initialUrl == creationParams.initialUrl && + MatchesWebSettings(_creationParams.webSettings) + .matches(creationParams.webSettings, matchState) && + orderedEquals(_creationParams.javascriptChannelNames) + .matches(creationParams.javascriptChannelNames, matchState); } } From 3b2bc5f8321f21d36f0296cd28832d52ffac4492 Mon Sep 17 00:00:00 2001 From: Amir Hardon Date: Wed, 22 May 2019 15:42:08 -0700 Subject: [PATCH 4/5] add more non null assertions --- packages/webview_flutter/lib/webview_flutter.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/webview_flutter/lib/webview_flutter.dart b/packages/webview_flutter/lib/webview_flutter.dart index 53fa9cf68296..ddea13dccef6 100644 --- a/packages/webview_flutter/lib/webview_flutter.dart +++ b/packages/webview_flutter/lib/webview_flutter.dart @@ -328,6 +328,9 @@ WebSettings _webSettingsUpdate(WebSettings currentValue, WebSettings newValue) { assert(currentValue.javascriptMode != null); assert(currentValue.hasNavigationDelegate != null); assert(currentValue.debuggingEnabled != null); + assert(newValue.javascriptMode != null); + assert(newValue.hasNavigationDelegate != null); + assert(newValue.debuggingEnabled != null); JavascriptMode javascriptMode; bool hasNavigationDelegate; bool debuggingEnabled; From b05a5c69cabbdf93e27dc397850c6297cec38996 Mon Sep 17 00:00:00 2001 From: Amir Hardon Date: Tue, 28 May 2019 11:45:17 -0700 Subject: [PATCH 5/5] review comment followup --- packages/webview_flutter/lib/webview_flutter.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/webview_flutter/lib/webview_flutter.dart b/packages/webview_flutter/lib/webview_flutter.dart index ddea13dccef6..a6126d709531 100644 --- a/packages/webview_flutter/lib/webview_flutter.dart +++ b/packages/webview_flutter/lib/webview_flutter.dart @@ -324,7 +324,8 @@ WebSettings _webSettingsFromWidget(WebView widget) { } // This method assumes that no fields in `currentValue` are null. -WebSettings _webSettingsUpdate(WebSettings currentValue, WebSettings newValue) { +WebSettings _clearUnchangedWebSettings( + WebSettings currentValue, WebSettings newValue) { assert(currentValue.javascriptMode != null); assert(currentValue.hasNavigationDelegate != null); assert(currentValue.debuggingEnabled != null); @@ -525,7 +526,8 @@ class WebViewController { } Future _updateSettings(WebSettings newSettings) { - final WebSettings update = _webSettingsUpdate(_settings, newSettings); + final WebSettings update = + _clearUnchangedWebSettings(_settings, newSettings); _settings = newSettings; return _platformInterface.updateSettings(update); }