-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[WIP] Transition Platform Implementation to Pigeon API #4455
Conversation
|
Where are the unit tests for webview_widget.dart? We should have unit tests for the Dart implementation just as we've been adding native unit tests for native implementations. Integration tests are useful, but shouldn't be a replacement. |
| /// an exception. | ||
| Future<void> release() { | ||
| final WebViewClient? webViewClient = _currentWebViewClient; | ||
| if (webViewClient != null) { |
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.
In what case would these be null before calling release()?
|
|
||
| final DownloadListener? downloadListener = _currentDownloadListener; | ||
| if (downloadListener != null) { | ||
| DownloadListener.api.disposeFromInstance(downloadListener); |
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.
Is there any way to push this logic into another method since it's repeated?
| sdk: flutter | ||
| webview_flutter_platform_interface: ^1.0.0 | ||
| webview_flutter_android: ^2.0.13 | ||
| webview_flutter_android: 2.0.13 |
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.
Unit tests fail because the default target platform is Android. Which now makes unmocked pigeon calls on creation. This can be fixed in a follow up PR to update tests to always run with a test WebViewPlatform.
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.
Couldn't we land that change first, instead of having to temporarily pin the version?
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.
That's probably the best solution. I'll create a PR for this now.
|
@stuartmorgan @camsim99 @mvanbeusekom @blasten This PR has became too big and does too many different things. I split this PR into 4 separate ones. Here are the first 3: |
This transitions the Android implementation of the platform interface to mostly Dart code.
Generated Files:
packages/webview_flutter/webview_flutter_android/test/webview_widget_test.mocks.dartpackages/webview_flutter/webview_flutter_android/test/android_webview_test.mocks.dartpackages/webview_flutter/webview_flutter_android/lib/src/android_webview.pigeon.dartpackages/webview_flutter/webview_flutter_android/test/android_webview.pigeon.dartPre-launch Checklist
dart format.)[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.