Implement LocationManagerLocationProvider with multi-provider registration and guardrails

[temcguir]

Summary

Implements a platform LocationProvider backed by LocationManagerCompat in :core:location:location-manager.

This is Part 2 of the location series, stacked on #586.

How it works

While location updates run, the provider listens to GPS and network location (and the fused provider on API 31+). Each accepted fix updates an in-memory cached location. Listening stops once a fix is within 50 meters or after 1 minute, and restarts after 5 minutes. If listening could not start (no permission, location off, or no provider available), it retries after 15 seconds. getCurrentLocation() returns the cached location without waiting on location hardware.

Starting updates while the preview is visible and tagging captures with the cached location are added in #588.

Key Changes

• :core:location:location-manager:
  • Implements LocationManagerLocationProvider:
    • Concurrently registers available platform providers (GPS_PROVIDER, NETWORK_PROVIDER, and FUSED_PROVIDER on API 31+) to ensure fast acquisition indoors and outdoors.
    • Enforces runtime permission checks and device master location state (isLocationAvailable()), returning null when location services are turned off even if permissions remain granted.
    • Restricts coarse-only permission queries to avoid attempting fine-only platform queries (GPS_PROVIDER and PASSIVE_PROVIDER).
    • Implements time-aware isBetterLocation() heuristics:
      • Fixes >2 minutes newer supersede older fixes regardless of accuracy, preventing coordinate anchoring while traveling.
      • Within the 2-minute window, fixes must be more accurate, newer and at least as accurate, or newer from the same provider and no more than 20 meters less accurate.
      • Fixes that do not report accuracy rank below any fix that does, and never end the update session early.
    • Evicts stale fixes older than 30 minutes from the cache and incoming updates.
    • Applies a 60-second hardware timeout to conserve battery when a fix <= 50 meters is not achieved, and automatically stops updates early once a fix <= 50 meters is accepted into the cache.
    • Repeats the location update session every 5 minutes during extended preview sessions to refresh coordinates, and retries after 15 seconds when a session could not start.
    • Filters out NaN, infinite, and (0, 0) coordinates.
• Testing:
  • Adds 37 Robolectric unit tests in LocationManagerLocationProviderTest covering multi-provider registration, anchor breaking, coarse-only filtering, permission revocation with cached fixes, out-of-order rejection, periodic refresh, retrying after permission is granted or location is enabled mid-session, and the precise location declaration log.

Permissions

• The module manifest declares ACCESS_COARSE_LOCATION, the minimum the provider needs. Apps that include the module receive it through manifest merging.
• To obtain precise location, an app declares ACCESS_FINE_LOCATION in its own manifest and requests it at runtime together with ACCESS_COARSE_LOCATION. No other integration is needed: permissions are re-checked on every update cycle, and the GPS provider is registered once precise location is granted.
• Declaring ACCESS_FINE_LOCATION later does not upgrade existing approximate grants.
• The provider logs once at INFO level when ACCESS_FINE_LOCATION is not declared, so approximate-only behavior is visible to integrators.
• MissingPermission suppressions are limited to removeUpdates, which must succeed even after permission is revoked. The remaining calls are guarded by explicit permission checks and SecurityException handling.

[gemini-code-assist]

Code Review

This pull request introduces the :core:location:location-manager module, implementing LocationManagerLocationProvider to provide location updates using the platform's LocationManager via LocationManagerCompat, along with comprehensive Robolectric tests. Feedback on the implementation includes safely retrieving the LocationManager to prevent potential null pointer exceptions on unsupported devices, extracting magic numbers into constants, checking provider availability before querying last known locations to avoid exception overhead, and removing high-frequency debug logs to prevent log spam.

[davidjiagoogle]

[Privacy / Permission Downgrade Edge Case] getCurrentLocation() can return a cached fine (GPS_PROVIDER) fix after permission is downgraded to Coarse-only

isLocationAvailable() checks hasAnyLocationPermission(), which returns true when either fine or coarse location is granted.

If the user initially grants Precise (Fine) location (populating cachedLocation with a high-accuracy GPS_PROVIDER fix) and then downgrades the app in system settings to Approximate (Coarse) only (ACCESS_FINE_LOCATION revoked, ACCESS_COARSE_LOCATION granted), getCurrentLocation() will continue returning the cached precise GPS_PROVIDER fix for up to 30 minutes (STALE_LOCATION_THRESHOLD_NANOS).

Suggestion: Check if (location.provider == LocationManager.GPS_PROVIDER && !hasFinePermission()) before returning the cached location (and fall back to getBestLastKnownLocation(), which already guards GPS_PROVIDER behind hasFinePermission()).

[temcguir]

I don't think this case can happen. Changing precise to approximate in system settings revokes ACCESS_FINE_LOCATION, and revoking a runtime permission restarts the app process, so the in-memory cache is gone. The provider check also wouldn't be complete on its own: with fine permission, FUSED and NETWORK fixes can be precise too, so filtering only GPS_PROVIDER wouldn't make the result coarse. Leaving it as is.

[davidjiagoogle]

[Behavior / Responsiveness] runLocationUpdates() delays for refreshIntervalMs (5 minutes) even when runUpdateSession() exits early without registering

If runUpdateSession() returns early because activeProviders.isEmpty() or permissions were not yet granted (for example, if the user had system location turned off in Quick Settings when opening the camera and turns it back on a few seconds later), the loop still waits the full refreshIntervalMs (5 minutes) before attempting to register hardware listeners again.

Suggestion: Consider having runUpdateSession() return a Boolean indicating whether a session actually ran (and using a shorter retry delay when no providers were available), or ensuring callers restart runLocationUpdates() when permission/location state changes.

[temcguir]

Fixed in ee158f0. runUpdateSession now returns whether a session ran, and the loop waits 15 seconds instead of the refresh interval when it didn't (no permission, location off, or no providers registered). Added tests for permission granted and system location turned on mid-session, and one that checks the refresh interval is still used after a completed session.

[davidjiagoogle]

[Race Condition / Ordering] isUpdating.set(true) is set after the listener registration loop

In runUpdateSession(), isUpdating.set(true) (line 158) is set only after the for loop finishes registering all providers. Meanwhile, in handleLocationUpdate() (line 240), stopHardwareUpdates() is called directly and checks if (isUpdating.getAndSet(false)).

If the first registered provider delivers an accurate fix synchronously/immediately before the registration loop completes (while isUpdating is still false), stopHardwareUpdates() inside handleLocationUpdate() is a no-op, and line 158 subsequently sets isUpdating.set(true) (though accurateFixReceived.complete(Unit) will then wake await() and trigger the finally block).

Suggestion: Set isUpdating.set(true) before the registration loop (and reset it to false if registeredCount == 0), or remove the direct stopHardwareUpdates() call in handleLocationUpdate() and rely exclusively on accurateFixDeferred.get()?.complete(Unit) waking up withTimeoutOrNull to run finally { stopHardwareUpdates() } on a single deterministic path.

[temcguir]

The listeners are registered on the main looper and the registration loop doesn't suspend, so a callback can't run before isUpdating is set. I agree a single stop path is clearer though, so in ee158f0 handleLocationUpdate no longer calls stopHardwareUpdates and only completes the deferred, and the finally block is the only place updates are stopped.

[davidjiagoogle]

[Cleanup / Consistency] Redundant and inconsistent provider checks between getBestLastKnownLocation() and getActiveProviders()

In getBestLastKnownLocation():

• LocationManagerCompat.hasProvider(locationManager, LocationManager.FUSED_PROVIDER) is checked inside buildList and then checked again by .filter { LocationManagerCompat.hasProvider(locationManager, it) }.
• locationManager.isProviderEnabled(...) is checked for FUSED_PROVIDER, whereas GPS_PROVIDER, NETWORK_PROVIDER, and PASSIVE_PROVIDER do not check isProviderEnabled here (unlike getActiveProviders() on lines 344–364).

Suggestion: Extract a shared helper or align the provider availability checks across both methods.

[temcguir]

Fixed in ee158f0. FUSED is added on S+ and the list goes through a single hasProvider filter. I dropped the isProviderEnabled check for FUSED rather than adding it to the others, since getLastKnownLocation already returns null for a disabled provider.