-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Allow changing the webview's platform specific implementation #1618
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:async'; | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/gestures.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
|
|
||
| /// Interface for talking to the webview's platform implementation. | ||
| /// | ||
| /// An instance implementing this interface is passed to the `onWebViewPlatformCreated` callback that is | ||
| /// passed to [WebViewPlatformBuilder#onWebViewPlatformCreated]. | ||
| abstract class WebViewPlatform { | ||
| /// Loads the specified URL. | ||
| /// | ||
| /// If `headers` is not null and the URL is an HTTP URL, the key value paris in `headers` will | ||
| /// be added as key value pairs of HTTP headers for the request. | ||
| /// | ||
| /// `url` must not be null. | ||
| /// | ||
| /// Throws an ArgumentError if `url` is not a valid URL string. | ||
| Future<void> loadUrl( | ||
| String url, | ||
| Map<String, String> headers, | ||
| ) { | ||
| throw UnimplementedError( | ||
| "WebView loadUrl 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 | ||
| // method calls so we need to expose the webview ID. | ||
| // TODO(amirh): remove this before publishing this package. | ||
| int get id; | ||
| } | ||
|
|
||
| typedef WebViewPlatformCreatedCallback = void Function( | ||
| WebViewPlatform webViewPlatform); | ||
|
|
||
| /// Interface building a platform WebView implementation. | ||
| /// | ||
| /// [WebView#platformBuilder] controls the builder that is used by [WebView]. | ||
| /// [AndroidWebViewPlatform] and [CupertinoWebViewPlatform] are the default implementations | ||
| /// for Android and iOS respectively. | ||
| abstract class WebViewBuilder { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: still think you should move this to it's own file
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to extract it if this file ever becomes too big, I kind of like the idea of having a single file that we can point implementors of new platforms to where everything they need to implement is specified... If you feel strongly about it I'll split it now. |
||
| /// Builds a new WebView. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should have an extra space above the dart doc?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we? I wasn't aware that we enforce something like that, I can't find anything about it in the style guide, are we enforcing this convention?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I asked because it seems other places in this file is doing that. If it is not enforced then I think we are ok. However, I do think it is better to keep the style consistent throughout a file or even a package.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand, how can weI enforce that |
||
| /// | ||
| /// Returns a Widget tree that embeds the created webview. | ||
| /// | ||
| /// `creationParams` are the initial parameters used to setup the webview. | ||
| /// | ||
| /// `onWebViewPlatformCreated` will be invoked after the platform specific [WebViewPlatform] | ||
| /// implementation is created with the [WebViewPlatform] instance as a parameter. | ||
| /// | ||
| /// `gestureRecognizers` specifies which gestures should be consumed by the web view. | ||
| /// It is possible for other gesture recognizers to be competing with the web view on pointer | ||
| /// events, e.g if the web view is inside a [ListView] the [ListView] will want to handle | ||
| /// vertical drags. The web view will claim gestures that are recognized by any of the | ||
| /// recognizers on this list. | ||
| /// When `gestureRecognizers` is empty or null, the web view will only handle pointer events for gestures that | ||
| /// were not claimed by any other gesture recognizer. | ||
| Widget build({ | ||
| BuildContext context, | ||
| // 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<String, dynamic> creationParams, | ||
| WebViewPlatformCreatedCallback onWebViewPlatformCreated, | ||
| Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers, | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/gestures.dart'; | ||
| import 'package:flutter/services.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
|
|
||
| import '../platform_interface.dart'; | ||
| import 'webview_method_channel.dart'; | ||
|
|
||
| /// Builds an Android webview. | ||
| /// | ||
| /// This is used as the default implementation for [WebView.platformBuilder] on Android. It uses | ||
| /// an [AndroidView] to embed the webview in the widget hierarchy, and uses a method channel to | ||
| /// communicate with the platform code. | ||
| class AndroidWebViewBuilder implements WebViewBuilder { | ||
| @override | ||
| Widget build({ | ||
| BuildContext context, | ||
| Map<String, dynamic> creationParams, | ||
| WebViewPlatformCreatedCallback onWebViewPlatformCreated, | ||
| Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers, | ||
| }) { | ||
| return GestureDetector( | ||
| // We prevent text selection by intercepting the long press event. | ||
| // This is a temporary stop gap due to issues with text selection on Android: | ||
| // https://github.com/flutter/flutter/issues/24585 - the text selection | ||
| // dialog is not responding to touch events. | ||
| // https://github.com/flutter/flutter/issues/24584 - the text selection | ||
| // handles are not showing. | ||
| // TODO(amirh): remove this when the issues above are fixed. | ||
| onLongPress: () {}, | ||
| excludeFromSemantics: true, | ||
| child: AndroidView( | ||
| viewType: 'plugins.flutter.io/webview', | ||
| onPlatformViewCreated: (int id) { | ||
| if (onWebViewPlatformCreated == null) { | ||
| return; | ||
| } | ||
| onWebViewPlatformCreated(MethodChannelWebViewPlatform(id)); | ||
| }, | ||
| gestureRecognizers: gestureRecognizers, | ||
| // WebView content is not affected by the Android view's layout direction, | ||
| // we explicitly set it here so that the widget doesn't require an ambient | ||
| // directionality. | ||
| layoutDirection: TextDirection.rtl, | ||
| creationParams: creationParams, | ||
| creationParamsCodec: const StandardMessageCodec(), | ||
| ), | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/gestures.dart'; | ||
| import 'package:flutter/services.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
|
|
||
| import '../platform_interface.dart'; | ||
| import 'webview_method_channel.dart'; | ||
|
|
||
| /// Builds an iOS webview. | ||
| /// | ||
| /// This is used as the default implementation for [WebView.platformBuilder] on iOS. It uses | ||
| /// a [UiKitView] to embed the webview in the widget hierarchy, and uses a method channel to | ||
| /// communicate with the platform code. | ||
| class CupertinoWebViewBuilder implements WebViewBuilder { | ||
| @override | ||
| Widget build({ | ||
| BuildContext context, | ||
| Map<String, dynamic> creationParams, | ||
| WebViewPlatformCreatedCallback onWebViewPlatformCreated, | ||
| Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers, | ||
| }) { | ||
| return UiKitView( | ||
| viewType: 'plugins.flutter.io/webview', | ||
| onPlatformViewCreated: (int id) { | ||
| if (onWebViewPlatformCreated == null) { | ||
| return; | ||
| } | ||
| onWebViewPlatformCreated(MethodChannelWebViewPlatform(id)); | ||
| }, | ||
| gestureRecognizers: gestureRecognizers, | ||
| creationParams: creationParams, | ||
| creationParamsCodec: const StandardMessageCodec(), | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:async'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing copyright statement
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
|
||
| import 'package:flutter/services.dart'; | ||
|
|
||
| import '../platform_interface.dart'; | ||
|
|
||
| /// A [WebViewPlatform] that uses a method channel to control the webview. | ||
| class MethodChannelWebViewPlatform implements WebViewPlatform { | ||
| MethodChannelWebViewPlatform(this._id) | ||
| : _channel = MethodChannel('plugins.flutter.io/webview_$_id'); | ||
|
|
||
| final int _id; | ||
|
|
||
| final MethodChannel _channel; | ||
|
|
||
| @override | ||
| Future<void> loadUrl( | ||
| String url, | ||
| Map<String, String> headers, | ||
| ) async { | ||
| assert(url != null); | ||
| // 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('loadUrl', <String, dynamic>{ | ||
| 'url': url, | ||
| 'headers': headers, | ||
| }); | ||
| } | ||
|
|
||
| @override | ||
| int get id => _id; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,10 @@ import 'package:flutter/gestures.dart'; | |
| import 'package:flutter/services.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
|
|
||
| import 'platform_interface.dart'; | ||
| import 'src/webview_android.dart'; | ||
| import 'src/webview_cupertino.dart'; | ||
|
|
||
| typedef void WebViewCreatedCallback(WebViewController controller); | ||
|
|
||
| enum JavascriptMode { | ||
|
|
@@ -121,6 +125,39 @@ class WebView extends StatefulWidget { | |
| }) : assert(javascriptMode != null), | ||
| super(key: key); | ||
|
|
||
| static WebViewBuilder _platformBuilder; | ||
|
|
||
| /// Sets a custom [WebViewBuilder]. | ||
| /// | ||
| /// This property can be set to use a custom platform implementation for WebViews. | ||
| /// | ||
| /// Setting `platformBuilder` doesn't affect [WebView]s that were already created. | ||
| /// | ||
| /// The default value is [AndroidWebViewBuilder] on Android and [CupertinoWebViewBuilder] on iOs. | ||
| static set platformBuilder(WebViewBuilder platformBuilder) { | ||
| _platformBuilder = platformBuilder; | ||
| } | ||
|
|
||
| /// The [WebViewBuilder] that's used to create new [WebView]s. | ||
| /// | ||
| /// The default value is [AndroidWebViewBuilder] on Android and [CupertinoWebViewBuilder] on iOs. | ||
| static WebViewBuilder get platformBuilder { | ||
| if (_platformBuilder == null) { | ||
| switch (defaultTargetPlatform) { | ||
| case TargetPlatform.android: | ||
| _platformBuilder = AndroidWebViewBuilder(); | ||
| break; | ||
| case TargetPlatform.iOS: | ||
| _platformBuilder = CupertinoWebViewBuilder(); | ||
| break; | ||
| default: | ||
| throw UnsupportedError( | ||
| "Trying to use the default webview implementation for $defaultTargetPlatform but there isn't a default one"); | ||
| } | ||
| } | ||
| return _platformBuilder; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you intend to cache the returned value?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
|
||
| /// If not null invoked once the web view is created. | ||
| final WebViewCreatedCallback onWebViewCreated; | ||
|
|
||
|
|
@@ -229,40 +266,12 @@ class _WebViewState extends State<WebView> { | |
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| if (defaultTargetPlatform == TargetPlatform.android) { | ||
| return GestureDetector( | ||
| // We prevent text selection by intercepting the long press event. | ||
| // This is a temporary stop gap due to issues with text selection on Android: | ||
| // https://github.com/flutter/flutter/issues/24585 - the text selection | ||
| // dialog is not responding to touch events. | ||
| // https://github.com/flutter/flutter/issues/24584 - the text selection | ||
| // handles are not showing. | ||
| // TODO(amirh): remove this when the issues above are fixed. | ||
| onLongPress: () {}, | ||
| excludeFromSemantics: true, | ||
| child: AndroidView( | ||
| viewType: 'plugins.flutter.io/webview', | ||
| onPlatformViewCreated: _onPlatformViewCreated, | ||
| gestureRecognizers: widget.gestureRecognizers, | ||
| // WebView content is not affected by the Android view's layout direction, | ||
| // we explicitly set it here so that the widget doesn't require an ambient | ||
| // directionality. | ||
| layoutDirection: TextDirection.rtl, | ||
| creationParams: _CreationParams.fromWidget(widget).toMap(), | ||
| creationParamsCodec: const StandardMessageCodec(), | ||
| ), | ||
| ); | ||
| } else if (defaultTargetPlatform == TargetPlatform.iOS) { | ||
| return UiKitView( | ||
| viewType: 'plugins.flutter.io/webview', | ||
| onPlatformViewCreated: _onPlatformViewCreated, | ||
| gestureRecognizers: widget.gestureRecognizers, | ||
| creationParams: _CreationParams.fromWidget(widget).toMap(), | ||
| creationParamsCodec: const StandardMessageCodec(), | ||
| ); | ||
| } | ||
| return Text( | ||
| '$defaultTargetPlatform is not yet supported by the webview_flutter plugin'); | ||
| return WebView.platformBuilder.build( | ||
| context: context, | ||
| creationParams: _CreationParams.fromWidget(widget).toMap(), | ||
| onWebViewPlatformCreated: _onWebViewPlatformCreated, | ||
| gestureRecognizers: widget.gestureRecognizers, | ||
| ); | ||
| } | ||
|
|
||
| @override | ||
|
|
@@ -279,8 +288,9 @@ class _WebViewState extends State<WebView> { | |
| (WebViewController controller) => controller._updateWidget(widget)); | ||
| } | ||
|
|
||
| void _onPlatformViewCreated(int id) { | ||
| final WebViewController controller = WebViewController._(id, widget); | ||
| void _onWebViewPlatformCreated(WebViewPlatform platformController) { | ||
| final WebViewController controller = | ||
| WebViewController._(platformController.id, platformController, widget); | ||
| _controller.complete(controller); | ||
| if (widget.onWebViewCreated != null) { | ||
| widget.onWebViewCreated(controller); | ||
|
|
@@ -385,6 +395,7 @@ class _WebSettings { | |
| class WebViewController { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the long term plan to get rid of this class in favor of WebViewPlatform?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the public API we expose to our users(vs the public Also note that renaming WebViewController right now will be a breaking change, I guess we could eventually make WebViewController extend WebViewPlatform if we want to save one indirection layer, and we extract all the utility code(like the logic in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that this should not be part of this PR, but in general I think this controller and WebViewPlatform going to look fairly similar so it would be nice if we can unify them
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's re-discuss it after we move all the methodchannel stuff out of WebViewController? I'm actually not sure which option I prefer, I can imagine having a value in separating the interface that users are using to control the webview from the interface that platforms need to provide for the plugin to control the webview (e.g I can see us adding some logic in WebViewController that don't belong in the platform implementation). We should decide at the end of this PR series before publishing to avoid a breaking change. |
||
| WebViewController._( | ||
| int id, | ||
| this._platformInterface, | ||
| this._widget, | ||
| ) : _channel = MethodChannel('plugins.flutter.io/webview_$id') { | ||
| _settings = _WebSettings.fromWidget(_widget); | ||
|
|
@@ -394,6 +405,8 @@ class WebViewController { | |
|
|
||
| final MethodChannel _channel; | ||
|
|
||
| final WebViewPlatform _platformInterface; | ||
|
|
||
| _WebSettings _settings; | ||
|
|
||
| WebView _widget; | ||
|
|
@@ -434,6 +447,9 @@ class WebViewController { | |
|
|
||
| /// Loads the specified URL. | ||
| /// | ||
| /// If `headers` is not null and the URL is an HTTP URL, the key value paris in `headers` will | ||
| /// be added as key value pairs of HTTP headers for the request. | ||
| /// | ||
| /// `url` must not be null. | ||
| /// | ||
| /// Throws an ArgumentError if `url` is not a valid URL string. | ||
|
|
@@ -443,13 +459,7 @@ class WebViewController { | |
| }) async { | ||
| assert(url != null); | ||
| _validateUrlString(url); | ||
| // 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('loadUrl', <String, dynamic>{ | ||
| 'url': url, | ||
| 'headers': headers, | ||
| }); | ||
| return _platformInterface.loadUrl(url, headers); | ||
| } | ||
|
|
||
| /// Accessor to the current URL that the WebView is displaying. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we enforce the specific platform implementation to honor the parameter constraints mentioned here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we can enforce it. The closet thing we have is the e2e test which we should hope implementors of new platforms will run.