[webview_flutter] Adds the loadFlutterAsset method to the interface.
#4562
Conversation
Adds the `loadFlutterAsset` method to the platform interface. This method, when implemented, is responsible for correctly loading HTML content that is part of the Flutter asset bundle.
mvanbeusekom
changed the title
loadFlutterAsset method to the interface.loadFlutterAsset method to the interface.
github-actions
bot
added
the
p: webview_flutter
| @override | ||
| Future<void> loadFlutterAsset(String key) async { | ||
| assert(key.isNotEmpty); | ||
| return _channel.invokeMethod<void>('loadFlutterAsset', key); |
There was a problem hiding this comment.
I missed this before with loadFile; for both of these methods, the docs claim that they throw ArgumentErrors in specific cases, but there's no mechanism for that here. Doesn't this method channel implementation need to have conversion logic that translates a specific error (e.g., by string matching on the error code) into an ArgumentError?
There was a problem hiding this comment.
That is a good catch, I missed that completely.
Actually I need to look into this as I tested on Android where is maps to the Android WebView.loadUrl method which will simply load a 404 page not found page and doesn't throw any exception at all. If the behaviour on iOS is similar maybe it would be better to remove the documentation on throwing an ArgumentException.
There was a problem hiding this comment.
On iOS we'd know that the resource lookup didn't return a path. Could we do that check on Android before passing the URL to the webview?
Changing the documentation would be okay, but I'd rather not have wildly different behavior on Android vs other platforms.
There was a problem hiding this comment.
I ran some tests and on iOS an error is reported through the WebViewPlatformCallbacksHandler.onWebResourceError callback. On Android no error is reported and it just loads the default 404 page.
I guess for the loadFile which excepts a local path we can simply check the file system if the file exists using dart:io. For the loadFlutterAsset method this will be more difficult unless there is a way we can verify (on the Dart side) that the supplied asset exists. I am not sure there is a good solution otherwise.
There was a problem hiding this comment.
unless there is a way we can verify (on the Dart side)
Why does it have to be on the Dart side? I meant querying the engine API to resolve a resource. We can expose that capability to the Dart code via method channel.
There was a problem hiding this comment.
The problem is that on the Android the native side only exposes the loadUrl and loadData methods (as it matches the native WebView API). This means on the Android side we cannot distinguish between the Dart loadFile, loadFlutterAsset and loadUrl methods as they all call the loadUrl method.
How would we go about querying the engine API? I don't have any experience with that.
There was a problem hiding this comment.
Hm, it looks like the engine always gives an answer on Android by constructing the path that it would be at, without actually checking that it's there. But I assume this would return null or [] for an asset that doesn't exist?
There was a problem hiding this comment.
That makes sense, I will try to confirm this tonight. If this works, would it then be appropriate to create a separate method channel call to expose this to Dart within the webview_flutter_android package?
There was a problem hiding this comment.
Yes, that seems like the right approach given the structure of all the other calls in the current implementation.
There was a problem hiding this comment.
I have updated the current implementation to correctly convert the PlatformException into a ArgumentError as mentioned in the documentation. Not sure if you want to give it another look since this change adds some additional code (including tests).
Converts the `PlatformException` into an `ArgumentException` when there is a failure loading the file or asset.
…interface_load_assets
| try { | ||
| return await _channel.invokeMethod<void>('loadFlutterAsset', key); | ||
| } on PlatformException catch (ex) { | ||
| if (ex.code == 'loadFlutterAsset_failed') { |
There was a problem hiding this comment.
Since we don't have to be compatible with an existing implementation (unlike loadFile) let's give this a specific code like loadFlutterAsset_invalidKey. If we ever have failure cases that aren't due to a bad key for some reason, we would't want them mapped to ArgumentError.
| ), | ||
| ), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Please also add tests for both that errors with different codes aren't remapped to ArgumentErrors.
…interface_load_assets
mvanbeusekom
added
the
waiting for tree to go green
Adds the
loadFlutterAssetmethod to the platform interface. This method, when implemented, is responsible for correctly loading HTML content that is part of the Flutter asset bundle.flutter/flutter#27086
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style].///).