Skip to content

[webview_flutter_wkwebview] Change callback methods with a non-null return type to non-null #8564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Feb 13, 2025

Conversation

bparrishMines
Copy link
Contributor

@bparrishMines bparrishMines commented Feb 4, 2025

Callback methods that can't return nullable value are made non-null. If the error seen by issues listed below, this should make the failure happen in Dart.

Related:
flutter/flutter#162437
flutter/flutter#162368

Fixes flutter/flutter#125901

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@bparrishMines bparrishMines changed the title Wkwebview improve [webview_flutter_wkwebview] Change callback methods with a non-null return type to non-null Feb 4, 2025
@bparrishMines bparrishMines marked this pull request as ready for review February 5, 2025 17:51
@bparrishMines bparrishMines requested review from stuartmorgan-g and removed request for cbracken February 5, 2025 17:51
@bparrishMines
Copy link
Contributor Author

This is ready for review, but I won't land this until the pigeon update is made: #8567

completion(.success(Void()))
}
}
print(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This codepath prints an error to the console but never calls completion, which seems like a bug. Doesn't the proxy API generator need to call completion(.failure(...)) in this path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a really good catch. I'll add this fix to #8567. It looks like Kotlin needs this too.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I'm not sure why we're suddenly getting a deprecation warning in PreferencesProxyAPIDelegate.swift since that code didn't change, but it looks like you'll need to add warning suppressions to that.

@@ -1,3 +1,8 @@
## 3.18.1

* Updates internal API wrapper to make all Flutter methods with a non-null return value be required
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would mention that this fixes a crash, since that's the client-visible effect of the change.

@stuartmorgan-g
Copy link
Contributor

Related:
flutter/flutter#162437
flutter/flutter#162368

Does this not fix that crash?

@stuartmorgan-g
Copy link
Contributor

(It shouldn't delay this PR since we should get this fix out ASAP, but consider a follow-up that adds an integration test that would have hit this crash, so we have higher-level testing of the potential delegate states as well.)

@bparrishMines
Copy link
Contributor Author

I'm not sure if it fixes the crash since I wasn't able to reproduce it. This should at least make it easier to debug since it will narrow down the cause.

@bparrishMines bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 13, 2025
@auto-submit auto-submit bot merged commit 26dcec3 into flutter:main Feb 13, 2025
82 checks passed
@bparrishMines bparrishMines deleted the wkwebview_improve branch February 13, 2025 18:30
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 17, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Feb 17, 2025
flutter/packages@625023a...8542af3

2025-02-15 [email protected] [various] Enable `permissive-` for
Windows plugin examples (flutter/packages#8636)
2025-02-15 [email protected] [pigeon] Update task queue handling
(flutter/packages#8627)
2025-02-14 [email protected] Update CODEOWNERS
(flutter/packages#8628)
2025-02-14 [email protected]
[flutter_adaptive_scaffold] Fix some memory leaks
(flutter/packages#8546)
2025-02-14 [email protected] [camera] Fix crash when
setting activeFormat on FLTCaptureDevice (flutter/packages#8630)
2025-02-13 [email protected] [camera] Remove remaining
OCMock usage in tests (flutter/packages#8624)
2025-02-13 [email protected]
[webview_flutter_wkwebview] Change callback methods with a non-null
return type to non-null (flutter/packages#8564)
2025-02-12 [email protected]
[google_sign_in_ios] Adds Swift Package Manager support
(flutter/packages#7356)
2025-02-12 [email protected] [camera_avfoundation] Migrate
tests to Swift - part 1 (flutter/packages#8603)
2025-02-12 [email protected] [video_player]
Re-enables `asset videos live stream duration != 0` test for Android
(flutter/packages#8610)
2025-02-12 [email protected]
[go_router_builder] Add support for `TypedStatefulShellBranch`'s
`preload` (flutter/packages#8587)
2025-02-12 [email protected] [local_auth_darwin] Fix test name for
clarity (flutter/packages#8499)
2025-02-11 [email protected] [ci] Manually roll master, set -Xmx4G
(flutter/packages#8586)
2025-02-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump the gradle-plugin group across 4 directories with 1
update (flutter/packages#8551)
2025-02-10 [email protected] [pigeon] Add
errors for ProxyAPI callback methods and null instances when reading in
a ProxyApiBaseCodec (flutter/packages#8567)
2025-02-10 [email protected]
[shared_preferences]Fix : SetState returning future
(flutter/packages#8398)
2025-02-10 [email protected] [various] Add deprecation notices to
READMEs (flutter/packages#8598)
2025-02-10 [email protected] [camera] Remove OCMock from
CameraSettingsTests, CameraMethodChannelTests and
CameraSessionPresetsTests (flutter/packages#8592)
2025-02-10 [email protected] [camera] Remove OCMock from
FLTCamPhotoCaptureTests, FLTSavePhotoDelegateTests and StreamingTests
(flutter/packages#8590)
2025-02-07 [email protected] [go_router]
Add `preload` parameter to `StatefulShellBranchData.$branch`
(flutter/packages#8545)
2025-02-07 [email protected] [video_player_avfoundation] iOS
platform view support (flutter/packages#8237)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: webview_flutter platform-ios platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[webview_flutter_wkwebview] -[WKPreferences javaScriptEnabled] is deprecated in iOS 14
2 participants