Skip to content

Commit c395a43

Browse files
nan-licursoragent
andcommitted
fix: only re-arm location updates when none is active to avoid churn
The early start() path now re-registers for location updates only when there is no active request, so a successful registration is left in place on repeated start() calls (e.g. toggling isShared or permission callbacks) instead of being cancelled and re-registered (which also reset the interval timer). Recovery after a failed first attempt is preserved because the flag is false in that case, and the focus-driven refresh path still re-registers unconditionally to pick up the new interval. Co-Authored-By: Cursor <cursoragent@cursor.com>
1 parent 59df183 commit c395a43

2 files changed

Lines changed: 10 additions & 14 deletions

File tree

OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/controller/impl/GmsLocationController.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,8 @@ internal class GmsLocationController(
5656
setLocationAndFire(localLastLocation)
5757
}
5858
}
59-
// Retry the subscription in case the first attempt failed because
60-
// permission was not yet granted, otherwise it would only recover on
61-
// the next app focus change.
62-
locationUpdateListener?.refreshRequest()
59+
// Re-register here if the first attempt failed
60+
locationUpdateListener?.refreshRequest(onlyIfInactive = true)
6361
wasSuccessful = true
6462
} else {
6563
try {
@@ -202,7 +200,7 @@ internal class GmsLocationController(
202200
}
203201
}
204202

205-
internal fun refreshRequest() {
203+
internal fun refreshRequest(onlyIfInactive: Boolean = false) {
206204
if (!googleApiClient.isConnected) {
207205
Logging.warn("Attempt to refresh location request but not currently connected!")
208206
return
@@ -224,6 +222,8 @@ internal class GmsLocationController(
224222

225223
synchronized(requestLock) {
226224
if (hasExistingRequest) {
225+
// Don't tear down an already-active request when only recovering a failed one.
226+
if (onlyIfInactive) return@synchronized
227227
_fusedLocationApiWrapper.cancelLocationUpdates(googleApiClient, this)
228228
}
229229
Logging.debug("GMSLocationController GoogleApiClient requestLocationUpdates!")

OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/controller/GmsLocationControllerTests.kt

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,8 @@ class GmsLocationControllerTests : FunSpec({
174174
}
175175
}
176176

177-
// Regression: when isShared is enabled before location permission is granted, the
178-
// GoogleApiClient connects (which does not require permission) but the first
179-
// requestLocationUpdates fails. A later start() (e.g. triggered by the permission grant)
180-
// previously short-circuited and only fired a one-shot last location, leaving the live
181-
// subscription dead until the next app focus change. It must now re-arm on that start().
182-
test("start re-arms the live location subscription on a subsequent start") {
177+
// Repeated start() on a healthy request must not cancel + re-register it (which resets the interval).
178+
test("start does not re-register an already-active request") {
183179
// Given
184180
val location = Location("TEST_PROVIDER")
185181
location.latitude = 123.45
@@ -194,11 +190,11 @@ class GmsLocationControllerTests : FunSpec({
194190
val response1 = gmsLocationController.start()
195191
val response2 = gmsLocationController.start()
196192

197-
// Then
193+
// Then the active request is left alone: not re-registered, not cancelled
198194
response1 shouldBe true
199195
response2 shouldBe true
200-
// Once on the initial init, and again when the second start re-arms the subscription.
201-
fusedLocationApiWrapperMock.requestLocationUpdatesCallCount shouldBe 2
196+
fusedLocationApiWrapperMock.requestLocationUpdatesCallCount shouldBe 1
197+
fusedLocationApiWrapperMock.cancelLocationUpdatesCallCount shouldBe 0
202198
}
203199

204200
// Regression: a requestLocationUpdates that fails (e.g. a swallowed SecurityException

0 commit comments

Comments
 (0)