Skip to content

Commit 6bb5222

Browse files
atavismjigar-f
andauthored
Fix Android VPN startup state and duplicate connect handling (#9085)
* fix(android): serialize VPN startup preparation * code review updates * code review updates * code review updates * code review updates * Notification: gate changes. (#9087) Co-authored-by: atavism <atavism@users.noreply.github.com> * code review updates --------- Co-authored-by: jigar-f <132374182+jigar-f@users.noreply.github.com>
1 parent f475d9a commit 6bb5222

9 files changed

Lines changed: 939 additions & 228 deletions

File tree

‎.github/workflows/android-compile-check.yml‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
name: Android Compile Check
22

3-
# Compiles all app Kotlin (by building the debug APK) on PRs that change Kotlin
4-
# source, to catch Kotlin compile breaks before they reach main. The full signed
3+
# Builds the debug APK and runs native unit tests on PRs that change Kotlin or
4+
# Gradle files, to catch regressions before they reach main. The full signed
55
# release build (build-android.yml) is workflow_call-only and never runs on PRs,
66
# so without this gate a Kotlin break (e.g. the bare success() call in #8754)
77
# could merge undetected.
88
#
9-
# Scope: the PR trigger covers Kotlin (*.kt) and shared CI setup changes. It
9+
# Scope: the PR trigger covers Kotlin, Android Gradle, and shared CI setup changes. It
1010
# does NOT run on Go/gomobile-AAR or Flutter/dep changes — a Go change that
1111
# breaks the AAR<->Kotlin interface would surface in build-android.yml /
1212
# release, not here. workflow_dispatch exists only for benchmarking/debugging.
@@ -18,6 +18,7 @@ on:
1818
pull_request:
1919
paths:
2020
- "**/*.kt"
21+
- "android/**/*.gradle"
2122
# keep the self-trigger so edits to this workflow re-run it
2223
- ".github/workflows/android-compile-check.yml"
2324
- ".github/actions/setup-android/**"
@@ -91,3 +92,7 @@ jobs:
9192
run: make android-debug-ci
9293
env:
9394
GOMOBILECACHE: ${{ env.GOMOBILECACHE }}
95+
96+
- name: Run Android unit tests
97+
working-directory: android
98+
run: ./gradlew :app:testDebugUnitTest

‎android/app/build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,8 @@ dependencies {
538538
implementation 'androidx.work:work-runtime-ktx:2.10.0'
539539
coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:2.1.5'
540540

541+
testImplementation 'junit:junit:4.13.2'
542+
541543
// Instrumentation harness for Flutter integration tests (Firebase Test Lab).
542544
// 1.2.0 matches what the integration_test Flutter plugin pins on the app
543545
// runtime classpath (AGP consistent resolution rejects anything newer).

‎android/app/src/main/kotlin/org/getlantern/lantern/MainActivity.kt‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,10 +233,19 @@ class MainActivity : FlutterFragmentActivity() {
233233
// Replays the pending selection after VPN consent, so consent doesn't fall back to auto.
234234
private fun resumePendingConnect() {
235235
val tag = pendingTag
236-
if (tag != null) {
237-
connectToServer(tag)
238-
} else {
239-
startVPN()
236+
runCatching {
237+
if (tag != null) {
238+
connectToServer(tag)
239+
} else {
240+
startVPN()
241+
}
242+
}.onFailure { e ->
243+
AppLogger.e(TAG, "Failed to start VPN after permission request", e)
244+
VpnStatusManager.postVPNError(
245+
errorCode = "start_vpn",
246+
errorMessage = "Error starting VPN service",
247+
error = e,
248+
)
240249
}
241250
}
242251

@@ -273,13 +282,15 @@ class MainActivity : FlutterFragmentActivity() {
273282
try {
274283
val intent = VpnService.prepare(this)
275284
if (intent != null) {
285+
VpnStatusManager.postVPNStatus(VPNStatus.Connecting)
276286
startActivityForResult(intent, VPN_PERMISSION_REQUEST_CODE)
277287
return false;
278288
} else {
279289
return true;
280290
}
281291
} catch (e: Exception) {
282292
AppLogger.e(TAG, "Error preparing VPN service", e)
293+
VpnStatusManager.postVPNStatus(VPNStatus.MissingPermission)
283294
return false
284295
}
285296
}

‎android/app/src/main/kotlin/org/getlantern/lantern/handler/MethodHandler.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,9 @@ class MethodHandler : FlutterPlugin,
204204
Methods.Start.method -> {
205205
scope.launch {
206206
result.runCatching {
207-
VpnStatusManager.postVPNStatus(VPNStatus.Connecting)
208207
MainActivity.instance.startVPN()
209208
success("VPN started")
210209
}.onFailure { e ->
211-
VpnStatusManager.postVPNStatus(VPNStatus.Disconnected)
212210
result.error("start_vpn", e.localizedMessage ?: "Please try again", e)
213211
}
214212
}

‎android/app/src/main/kotlin/org/getlantern/lantern/notification/NotificationManager.kt‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class NotificationHelper {
4545

4646
private lateinit var dataUsageNotificationChannel: NotificationChannel
4747
private lateinit var vpnNotificationChannel: NotificationChannel
48+
private var foregroundStarted = false
4849

4950

5051
init {
@@ -158,9 +159,16 @@ class NotificationHelper {
158159
/**
159160
* Shows the starting VPN notification as a foreground notification.
160161
* Also starts the service in the foreground and promotes it to a foreground service.
162+
*
163+
* @return true if this call promoted the service; false if it was already in the foreground.
161164
*/
162-
fun showStartingVPNConnectedNotification(vpnService: LanternVpnService) {
165+
@Synchronized
166+
fun showStartingVPNConnectedNotification(vpnService: LanternVpnService): Boolean {
167+
// Duplicate starts must not replace an existing connected notification.
168+
if (foregroundStarted) return false
163169
showForegroundNotification(vpnService, VPN_CONNECTED, buildStartingVpnNotification())
170+
foregroundStarted = true
171+
return true
164172
}
165173

166174
/**
@@ -184,13 +192,15 @@ class NotificationHelper {
184192
* @param service The service whose foreground status should be stopped.
185193
* @param removeNotification Whether to remove the notification from the status bar.
186194
*/
195+
@Synchronized
187196
private fun stopForegroundNotification(service: LanternVpnService) {
188197
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
189198
service.stopForeground(STOP_FOREGROUND_REMOVE)
190199
} else {
191200
// For API < 24, stopForeground without flags
192201
service.stopForeground(true)
193202
}
203+
foregroundStarted = false
194204
}
195205

196206

‎android/app/src/main/kotlin/org/getlantern/lantern/service/DefaultNetworkMonitor.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ object DefaultNetworkMonitor {
3030
// different threads, so @Volatile guarantees the read sees the latest write.
3131
@Volatile
3232
private var listener: InterfaceUpdateListener? = null
33+
@Volatile
3334
private var networkChangeCallback: ((Network?) -> Unit)? = null
3435

3536
/**

0 commit comments

Comments
 (0)