fix: [SDK-4672] start location updates when permission is granted

[nan-li]

Description

One Line Summary

When registering for ongoing location updates fails, the SDK now records the failure and registers again the next time location starts, instead of assuming the first attempt worked and never retrying.

Details

Motivation

If an app turns on location sharing (isShared = true) before the user has granted location permission, the SDK tries to register for ongoing location updates, but that call fails because the permission is missing. The failure was caught but not reported back, so the SDK recorded the location-updates listener as registered when it actually was not. After the user granted permission, the SDK sent a single saved location and did not register for ongoing updates again, so no further location changes were reported until the next time the app was sent to the background and brought back to the foreground.

This change makes the registration call report whether it actually succeeded, so the SDK no longer treats a failed attempt as registered, and it registers for ongoing updates again as soon as permission is granted. Ongoing updates now start right away.

Scope

• Affects the Google Play Services location path only; the Huawei path is unchanged.
• Ongoing updates now start at the moment permission is granted.
• No public API changes.

Testing

Unit testing

Added two tests in GmsLocationControllerTests: one checks that calling start() again registers for location updates a second time, and one checks that a failed registration is not recorded as active. All location unit tests and detekt pass.

Manual testing

Reproduced on the demo app (com.onesignal.example, built from this branch) on a Pixel 7 / Android 14 emulator: turned on isShared before granting permission, then chose "While using the app". Before the change, ongoing updates started only after the app was sent to the background and brought back; after the change, the SDK registers for ongoing updates right after the grant with no error, and the latitude/longitude reaches OneSignal in the same foreground session.

Verification logs

Turn on isShared before permission — the registration call fails, so no ongoing updates are started:

16:37:45.358 D LocationManager.setIsShared(value: true)
16:37:45.358 D LocationManager.startGetLocation()
16:37:45.388 D GMSLocationController GoogleApiClientListener onConnected
16:37:45.395 D GMSLocationController GoogleApiClient requestLocationUpdates!
16:37:45.398 W FusedLocationApi.requestLocationUpdates failed!
16:37:45.398 W java.lang.SecurityException: uid 10204 does not have any of
              [ACCESS_FINE_LOCATION, ACCESS_COARSE_LOCATION]

Grant "While using the app" — the SDK registers for ongoing updates immediately, with no background/foreground cycle in between:

16:38:15.663 D ...key=USER_RESOLVED_PERMISSION_android.permission.ACCESS_FINE_LOCATION
16:38:15.664 D LocationManager.startGetLocation()
16:38:15.676 D ApplicationService.onActivityResumed: MainActivity
16:38:15.684 D LocationController fireCompleteForLocation: Location[fused 37.421998,-122.084 ...]
16:38:15.695 D GMSLocationController GoogleApiClient requestLocationUpdates!
16:38:20.699 D HttpClient: PATCH .../users/...  Body: {"properties":{"lat":37.4219983,"long":-122.084}}
16:38:20.917 D HttpClient: PATCH ... STATUS: 202

The requestLocationUpdates! at the grant has no SecurityException after it, and there is no LocationUpdateListener.onFocus()/onUnfocused() between the grant and the re-registration — so ongoing updates start from the permission grant itself, not from a background/foreground cycle.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• No public API changes

Testing

• I have included test coverage for these changes
• All automated tests pass
• I have personally tested this on an emulator

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself

Made with Cursor

[nan-li]

@claude review

[github-actions]

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

• FusedLocationApiWrapperImpl.kt: 0/4 touched executable lines (0.0%) (6 touched lines in diff)
  • 4 uncovered touched lines in this file
• GmsLocationController.kt: 0/2 touched executable lines (0.0%) (6 touched lines in diff)
  • 2 uncovered touched lines in this file

Overall (aggregate gate)

0/6 touched executable lines covered (0.0% — requires ≥ 80%)

Per-file detail (informational; gate is aggregate above):

• FusedLocationApiWrapperImpl.kt: 0.0% (4 uncovered touched lines)

• GmsLocationController.kt: 0.0% (2 uncovered touched lines)

❌ Coverage Check Failed

Aggregate coverage on touched lines is 0.0% (minimum 80%).

📥 View workflow run

[claude]

LGTM — small, well-tested fix that re-arms the live location subscription when start() is called again after a permission grant.

Extended reasoning...

Overview

This PR fixes SDK-4672 by making location updates start immediately after the user grants permission, rather than waiting for the next app focus change. It modifies FusedLocationApiWrapperImpl.requestLocationUpdates to return a Boolean indicating success/failure, threads that result into LocationUpdateListener.hasExistingRequest so failed requests are not later cancelled as if they were active, and adds a refreshRequest() call in the start() re-entrance branch so a second start() (triggered by the permission grant) re-arms the subscription instead of short-circuiting. Visibility of refreshRequest was widened from private to internal, the mock was extended with call counters and a configurable return value, and two regression tests were added.

Security risks

None meaningful. The PR does not change how permissions are checked or requested — it only changes how the SDK reacts to an existing failed request once permission becomes available. No public API change, no data exposure, no auth surface.

Level of scrutiny

Low-to-moderate. The diff is confined to the GMS location subsystem (Huawei path untouched), the change is mechanical (propagate a boolean, retry on re-entrance), and existing tests plus two new regression tests cover both the happy path and the failed-request-not-treated-as-active edge case. The new refreshRequest() call sits inside the existing startStopMutex, so it inherits the same synchronization as the rest of start().

Other factors

The bug-hunting pass found no issues. The author reported manual reproduction and verification on an Android 14 emulator. The fix aligns cleanly with the prior commit 032afbe on the same ticket. No outstanding reviewer comments to address.

[fadi-george]

Is this similar to the other location pr?

[nan-li]

Is this similar to the other location pr?

No, that one catches throwables in LocationServices API only.

This one calls an API requestLocationUpdates that already is wrapped in a try-catch but it has to do with making sure the SDK actually subscribes to location updates instead of silently failing. I updated the Motivation section to be more clear.