fix(connectivity_plus): Ensure Connectivity on Android is correctly reported when lost

[miquelbeltran]

Description

• Doing tests, I noticed that the connectivity did not change when disabling mobile connection, see [Bug]: Turn off android mobile data returns ConnectivityResult.mobile when it should returns connectivityResult.none on HyperOS android 14 #2831 (comment)
  • The events received in onConnectivityChange were not correct.
  • When I disconnected mobile, the onConnectivityChange was still reporting connectivity mobile.
• I did some changes on the connectivity change listeners in Android and the issue is gone.
  • In the onLost callback: Increased the delay to 500 milliseconds to avoid race conditions.
  • In the other two callbacks, use the provided Network and Capabilities objects, this ensures that the actual event payload is used and not reading a value that may be not up-to-date.
  • The "delayed" callback is now only used onLost.
  • I added a distinct to the stream, because the callback was being called a lot of times after a change. I don't think users will miss that ;)
• Tested on Pixel 5, disconnecting wifi and/or mobile network, and it gets reported correctly.

What a plugin consumer will see:

1. Phone connected to wifi and mobile: [wifi] is emitted.
2. Phone disconnects from wifi and connects to mobile: [mobile] is emitted. (Note: Users may still see a [none] event if the phone fails to connect to mobile while switching)
3. Phone connects to wifi again: [wifi] is emitted.
4. All connection is lost: [none] is emitted.
5. etc.

With a VPN + wifi:

1. Phone connected to wifi + vpn: [wifi, vpn]
2. Phone disconnects from vpn: [wifi]

BTW, from the Android documentation:

Do NOT call ConnectivityManager.getNetworkCapabilities(android.net.Network) or ConnectivityManager.getLinkProperties(android.net.Network) or other synchronous ConnectivityManager methods in this callback as this is prone to race conditions ; calling these methods while in a callback may return an outdated or even a null object.

Which is why users were still seeing mobile connectivity even if it was disconnected.
The 100 ms delay we had in place was not enough to ensure that wasn't an issue.
This is why it is increased to 500 ms and used only in onLost since it is the only place where it is strictly necessary.

Related Issues

• Fix [Bug]: Turn off android mobile data returns ConnectivityResult.mobile when it should returns connectivityResult.none on HyperOS android 14 #2831
• Potentially other older tickets.

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I titled the PR using Conventional Commits.
• I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
• All existing and new tests are passing.
• The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
• No, this is not a breaking change.

[miquelbeltran]

Reading a bit more the Android documentation, I need to do some changes, as this is not fully correct still.

edit: done.

[vbuberen]

BTW, from the Android documentation:

Do NOT call ConnectivityManager.getNetworkCapabilities(android.net.Network) or ConnectivityManager.getLinkProperties(android.net.Network) or other synchronous ConnectivityManager methods in this callback as this is prone to race conditions ; calling these methods while in a callback may return an outdated or even a null object.

It is something I shared in this PR adding 100 ms delay: #2673 (comment)
I think it depends on device, because I had no issues on Pixel 7 and emulators.

[vbuberen]

I like that there is a distinct now, but I don't like that none in the middle due to onLost sending none status, because it is wrong for scenarios like you described in your vpn + wifi case.

I will also do some testing of these changes later

[vbuberen]

The events received in onConnectivityChange were not correct.
When I disconnected mobile, the onConnectivityChange was still reporting connectivity mobile.

In the PR that I already mentioned above there is a screen recording that disconnecting mobile works fine and updates status properly. It was on a real device. And same happened on emulator.

[miquelbeltran]

but I don't like that none in the middle due to onLost sending none status, because it is wrong for scenarios like you described in your vpn + wifi case.

Understandable, I think a middle ground solution would be do the delayed call to obtain the connectivity status in onLost, but keep my changes for the other two callbacks, so we ensure that onAvailable and onCapabilitiesChanged use the provided Network and NetworkCapabilities objects.

I'd also increase the delay time. I did see problems in my Android phone (Pixel 5) when switching off mobile network. WiFi works well, but with mobile networks the stream did emit a mobile connectivity even after disconnecting.

[miquelbeltran]

I have updated the implementation, the onLost callback now doesn't emit a [none], but rather uses the same postDelayed as before, except the delay is now 500 millis rather than 100. This ensures that the device has enough time to report the correct value.

[vbuberen]

I did some testing for various cases on my Pixel 7 and everything works as it should.
I think we would still see some devices with customised ROMs to have a similar issue, but hope that increased delay would help.