From 7dcc62a2a87d6bd2cdee1033c70c96230d73e231 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 13:51:45 -0400 Subject: [PATCH 1/7] feat(chat): live location watches natively and holds the app up for a fix Live location ran entirely through the chat UI: the service asked JS to watch position and JS relayed every fix back over RPC. With no JS, there was no location. startWatch now splits -- when a native LocationWatcher is wired up it starts the OS watch itself (ref-counted across trackers, so one watch serves them all) and only asks the chat UI for the permission prompt it cannot raise on its own. NativeLocationUpdate takes every fix the watcher reports and shouldRecordFix decides which to keep: in the foreground all of them, out of it only once the device has moved 65m since the last recorded one, measured with a haversine over the fix-to-fix path so a slow drift still adds up. This also restores what the lifecycle rework left open. A fix can wake a backgrounded app, and the update needs to get out before iOS suspends it again, so a fix opens a background-work hold; releaseHoldIfIdleLocked ends it once the last tracker is gone. Nothing writes MobileAppState directly any more. Three bugs fixed along the way. The chat-UI watch retry loop incremented maxWatchAttempts where it meant watchAttempts, so it never terminated. Its sleep went through time.Sleep instead of the injected clock, so it could not be tested. And lastCoord was read without the lock from tracker() and updateMapUnfurl(); it now goes through getLastCoord(). The errgroup is also per-run now, so Stop can't end up waiting on a tracker that started after it. --- go/chat/globals/globals.go | 1 + go/chat/maps/livelocation.go | 216 +++++++++++-- go/chat/maps/livelocation_appstate_test.go | 122 ++++++++ go/chat/maps/livelocation_throttle_test.go | 119 +++++++ go/chat/maps/livelocation_watch_test.go | 343 +++++++++++++++++++++ go/chat/types/interfaces.go | 10 + go/chat/unfurl/scraper_test.go | 4 + 7 files changed, 782 insertions(+), 33 deletions(-) create mode 100644 go/chat/maps/livelocation_appstate_test.go create mode 100644 go/chat/maps/livelocation_throttle_test.go create mode 100644 go/chat/maps/livelocation_watch_test.go diff --git a/go/chat/globals/globals.go b/go/chat/globals/globals.go index 911f042cd739..a5736f4babaf 100644 --- a/go/chat/globals/globals.go +++ b/go/chat/globals/globals.go @@ -34,6 +34,7 @@ type ChatContext struct { AttachmentUploader types.AttachmentUploader // upload attachments NativeVideoHelper types.NativeVideoHelper // connection to native for doing things with video ShareIntentDonator types.ShareIntentDonator // donate share sheet suggestions (iOS only) + LocationWatcher types.LocationWatcher // native location service for live location (iOS only) StellarLoader types.StellarLoader // stellar payment/request loader StellarSender types.StellarSender // stellar in-chat payment sender StellarPushHandler types.OobmHandler diff --git a/go/chat/maps/livelocation.go b/go/chat/maps/livelocation.go index 6b8f6fe24a38..5cb358d1a153 100644 --- a/go/chat/maps/livelocation.go +++ b/go/chat/maps/livelocation.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "sync" "time" @@ -12,6 +13,7 @@ import ( "github.com/keybase/client/go/chat/types" "github.com/keybase/client/go/chat/utils" "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/libkb/lifecycle" "github.com/keybase/client/go/protocol/chat1" "github.com/keybase/client/go/protocol/gregor1" "github.com/keybase/client/go/protocol/keybase1" @@ -28,10 +30,21 @@ type LiveLocationTracker struct { storage *trackStorage updateInterval time.Duration uid gregor1.UID - eg errgroup.Group trackers map[types.LiveLocationKey]*locationTrack lastCoord chat1.Coordinate maxCoords int + // eg runs the trackers started since the last Stop, which replaces it and + // waits on the old one: a tracker can start at any time, even before Start, + // and must not join a group that a Stop is already waiting on. + eg *errgroup.Group + // bgHold keeps the app running while tracking; guarded by the tracker's + // mutex and changed only by releaseHoldIfIdleLocked and + // ensureHoldOnFixLocked. + bgHold *lifecycle.Hold + + nativeWatchMu sync.Mutex + nativeWatchRefs int + fixThrottle fixThrottle // testing only TestingCoordsAddedCh chan struct{} @@ -47,6 +60,7 @@ func NewLiveLocationTracker(g *globals.Context) *LiveLocationTracker { updateInterval: 30 * time.Second, maxCoords: 500, clock: clockwork.NewRealClock(), + eg: new(errgroup.Group), } } @@ -69,8 +83,10 @@ func (l *LiveLocationTracker) Stop(ctx context.Context) chan struct{} { for _, t := range l.trackers { t.Stop() } + eg := l.eg + l.eg = new(errgroup.Group) go func() { - _ = l.eg.Wait() + _ = eg.Wait() close(ch) }() return ch @@ -92,6 +108,34 @@ func (l *LiveLocationTracker) saveLocked(ctx context.Context) { } } +func (l *LiveLocationTracker) removeTrackerLocked(ctx context.Context, t *locationTrack) { + delete(l.trackers, t.Key()) + l.saveLocked(ctx) + l.releaseHoldIfIdleLocked() +} + +// releaseHoldIfIdleLocked ends the hold once nothing is tracked. Every removal +// from the trackers map calls it. +func (l *LiveLocationTracker) releaseHoldIfIdleLocked() { + if len(l.trackers) == 0 && l.bgHold != nil { + l.bgHold.Release() + l.bgHold = nil + } +} + +// ensureHoldOnFixLocked opens a hold for a location fix, since the fix can +// wake a backgrounded app and the hold keeps it up until the update gets out. +// A hold the controller ended -- WillTerminate does, and nothing else -- is +// replaced, so a fix after one still gets the app held up. +func (l *LiveLocationTracker) ensureHoldOnFixLocked() { + if len(l.trackers) == 0 || !l.G().IsMobileAppType() { + return + } + if l.bgHold == nil || l.bgHold.Released() { + l.bgHold = l.G().MobileLifecycle.AcquireBackgroundWork() + } +} + func (l *LiveLocationTracker) restoreLocked(ctx context.Context) { trackers, err := l.storage.Restore(ctx) if err != nil { @@ -102,6 +146,10 @@ func (l *LiveLocationTracker) restoreLocked(ctx context.Context) { return } l.Debug(ctx, "restoreLocked: restored %d trackers", len(trackers)) + l.runRestoredLocked(trackers) +} + +func (l *LiveLocationTracker) runRestoredLocked(trackers []*locationTrack) { l.trackers = make(map[types.LiveLocationKey]*locationTrack) for _, t := range trackers { if t.IsStopped() { @@ -113,6 +161,16 @@ func (l *LiveLocationTracker) restoreLocked(ctx context.Context) { return l.tracker(myT) }) } + // The replacement above can drop a hold's only tracker without ever + // running removeTrackerLocked for it, so release directly here rather + // than leaving the app held until some later fix notices. + l.releaseHoldIfIdleLocked() +} + +func (l *LiveLocationTracker) getLastCoord() chat1.Coordinate { + l.Lock() + defer l.Unlock() + return l.lastCoord } func (l *LiveLocationTracker) getChatUI(ctx context.Context) libkb.ChatUI { @@ -185,8 +243,8 @@ func (l *LiveLocationTracker) updateMapUnfurl(ctx context.Context, t *locationTr var coords []chat1.Coordinate trackerCoords := t.GetCoords() if len(trackerCoords) == 0 { - if !l.lastCoord.IsZero() { - coords = []chat1.Coordinate{l.lastCoord} + if lastCoord := l.getLastCoord(); !lastCoord.IsZero() { + coords = []chat1.Coordinate{lastCoord} } else { return errors.New("no coordinates") } @@ -235,58 +293,99 @@ func (l *LiveLocationTracker) updateMapUnfurl(ctx context.Context, t *locationTr return nil } -func (l *LiveLocationTracker) startWatch(ctx context.Context, t *locationTrack) (watchID chat1.LocationWatchID, err error) { +// startWatch starts OS location updates for t and returns the function that +// ends them. +func (l *LiveLocationTracker) startWatch(ctx context.Context, t *locationTrack) (watchID chat1.LocationWatchID, stop func(), err error) { + if w := l.G().LocationWatcher; w != nil { + l.acquireNativeWatch(w) + // The native watcher only checks authorization, it can't prompt. The chat + // UI, when there is one, asks for permission and reports a failure in the + // conversation; with no UI this does nothing. + if _, err := l.getChatUI(ctx).ChatWatchPosition(ctx, t.convID, t.perm); err != nil { + l.Debug(ctx, "startWatch: unable to request location permission: %s", err) + } + return 0, func() { l.releaseNativeWatch(w) }, nil + } + watchID, err = l.startChatUIWatch(ctx, t) + if err != nil { + return 0, nil, err + } + return watchID, func() { + if err := l.getChatUI(ctx).ChatClearWatch(ctx, watchID); err != nil { + l.Debug(ctx, "tracker[%v]: error clearing watch: %+v", watchID, err) + } + }, nil +} + +// acquireNativeWatch and releaseNativeWatch share one native watch among all +// trackers. The watcher is called under the lock so it sees starts and stops +// in order. +func (l *LiveLocationTracker) acquireNativeWatch(w types.LocationWatcher) { + l.nativeWatchMu.Lock() + defer l.nativeWatchMu.Unlock() + l.nativeWatchRefs++ + if l.nativeWatchRefs == 1 { + l.fixThrottle = fixThrottle{} + w.StartWatching() + } +} + +func (l *LiveLocationTracker) releaseNativeWatch(w types.LocationWatcher) { + l.nativeWatchMu.Lock() + defer l.nativeWatchMu.Unlock() + l.nativeWatchRefs-- + if l.nativeWatchRefs == 0 { + w.StopWatching() + } +} + +func (l *LiveLocationTracker) startChatUIWatch(ctx context.Context, t *locationTrack) (watchID chat1.LocationWatchID, err error) { // try this a couple times in case we are starting fresh and the UI isn't ready yet maxWatchAttempts := 20 watchAttempts := 0 for { if watchID, err = l.getChatUI(ctx).ChatWatchPosition(ctx, t.convID, t.perm); err != nil { - l.Debug(ctx, "startWatch: unable to watch position: attempt: %d msg: %s", watchAttempts, err) + l.Debug(ctx, "startChatUIWatch: unable to watch position: attempt: %d msg: %s", watchAttempts, err) if watchAttempts > maxWatchAttempts { return 0, err } } else { break } - maxWatchAttempts++ - time.Sleep(time.Second) + watchAttempts++ + l.clock.Sleep(time.Second) } return watchID, nil } func (l *LiveLocationTracker) tracker(t *locationTrack) error { ctx := context.Background() - // check to see if we are being asked to start a tracker that is already expired - if t.endTime.Before(l.clock.Now()) { + // Every exit removes the tracker, which also ends the background-work hold + // once no tracker remains. + defer func() { l.Lock() defer l.Unlock() - delete(l.trackers, t.Key()) - l.saveLocked(ctx) + l.removeTrackerLocked(ctx, t) + }() + // check to see if we are being asked to start a tracker that is already expired + if t.endTime.Before(l.clock.Now()) { l.Debug(ctx, "tracker: old tracker, not running and clearing") return errors.New("tracker from the past") } // start up the OS watch routine - watchID, err := l.startWatch(ctx, t) + watchID, stopWatch, err := l.startWatch(ctx, t) if err != nil { + l.Debug(ctx, "tracker: unable to start watching, clearing: %s", err) return err } - defer func() { - // drop everything when our live location ends - err := l.getChatUI(ctx).ChatClearWatch(ctx, watchID) - if err != nil { - l.Debug(ctx, "tracker[%v]: error clearing watch: %+v", watchID, err) - } - l.Lock() - defer l.Unlock() - delete(l.trackers, t.Key()) - l.saveLocked(ctx) - }() + // Deferred after the removal, so it runs first: stop watching, then remove. + defer stopWatch() // if this is a live location request, just put whatever the last coord is on the screen, makes it // feel more live - if !l.lastCoord.IsZero() { + if lastCoord := l.getLastCoord(); !lastCoord.IsZero() { l.Debug(ctx, "tracker[%v]: updating with last coord", watchID) - t.updateCh <- l.lastCoord + t.updateCh <- lastCoord } firstUpdate := true shouldUpdate := false @@ -369,17 +468,68 @@ func (l *LiveLocationTracker) StartTracking(ctx context.Context, convID chat1.Co l.eg.Go(func() error { return l.tracker(t) }) } +// backgroundFixDistance is how far, in meters, the device must move before a +// native fix is recorded while the app is not in the foreground. +const backgroundFixDistance = 65 + +// earthRadiusMeters is the mean radius of the Earth. +const earthRadiusMeters = 6371008.8 + +// fixThrottle is what shouldRecordFix knows of the fixes since the native +// watch started. +type fixThrottle struct { + // prev is the latest fix, recorded or not; nil until the first one. + prev *chat1.Coordinate + // pendingDistance is how far the device has moved, fix to fix, since the + // last recorded fix. + pendingDistance float64 +} + +// shouldRecordFix decides whether a native fix gets recorded, and returns the +// throttle to use for the next one. Out of the foreground a fix is recorded +// only once the device has moved backgroundFixDistance since the last one +// recorded. The first fix after the watch starts is recorded right away, so the +// move that relaunched the app gets posted. +func shouldRecordFix(state keybase1.MobileAppState, last fixThrottle, next chat1.Coordinate) (bool, fixThrottle) { + if last.prev != nil { + last.pendingDistance += distanceMeters(*last.prev, next) + } + record := last.prev == nil || state == keybase1.MobileAppState_FOREGROUND || + last.pendingDistance >= backgroundFixDistance + last.prev = &next + if record { + last.pendingDistance = 0 + } + return record, last +} + +// distanceMeters is the great-circle distance between a and b. +func distanceMeters(a, b chat1.Coordinate) float64 { + rad := func(deg float64) float64 { return deg * math.Pi / 180 } + dLat := rad(b.Lat - a.Lat) + dLon := rad(b.Lon - a.Lon) + h := math.Sin(dLat/2)*math.Sin(dLat/2) + + math.Cos(rad(a.Lat))*math.Cos(rad(b.Lat))*math.Sin(dLon/2)*math.Sin(dLon/2) + return 2 * earthRadiusMeters * math.Asin(math.Min(1, math.Sqrt(h))) +} + +// NativeLocationUpdate takes a fix from the native location watcher, which +// reports every fix, and records the ones shouldRecordFix lets through. +func (l *LiveLocationTracker) NativeLocationUpdate(ctx context.Context, coord chat1.Coordinate) { + l.nativeWatchMu.Lock() + record, throttle := shouldRecordFix(l.G().MobileAppState.State(), l.fixThrottle, coord) + l.fixThrottle = throttle + l.nativeWatchMu.Unlock() + if record { + l.LocationUpdate(ctx, coord) + } +} + func (l *LiveLocationTracker) LocationUpdate(ctx context.Context, coord chat1.Coordinate) { defer l.Trace(ctx, nil, "LocationUpdate")() l.Lock() defer l.Unlock() - // A fix that arrives while the app is backgrounded no longer keeps the app - // out of BACKGROUND: the service derives its state from the UI reports - // native makes and the holds background work opens, and nothing may write - // the state directly any more. Tracking does not yet open a hold of its - // own, so a long background track can go quiet once the background task - // that started when the UI left the screen has ended. Deliberate and - // temporary -- the hold arrives with the live-location rework. + l.ensureHoldOnFixLocked() if l.lastCoord.Eq(coord) { l.Debug(ctx, "LocationUpdate: ignoring dup coordinate") return diff --git a/go/chat/maps/livelocation_appstate_test.go b/go/chat/maps/livelocation_appstate_test.go new file mode 100644 index 000000000000..843a5eed5e35 --- /dev/null +++ b/go/chat/maps/livelocation_appstate_test.go @@ -0,0 +1,122 @@ +package maps + +import ( + "context" + "testing" + "time" + + "github.com/keybase/client/go/chat/globals" + "github.com/keybase/client/go/protocol/chat1" + + "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/libkb/lifecycle/lifecycletest" + "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/require" +) + +func TestLiveLocationTrackerBackgroundActive(t *testing.T) { + t.Setenv("KEYBASE_APP_TYPE", string(libkb.MobileAppType)) + tc := libkb.SetupTest(t, "LiveLocationTrackerBackgroundActive", 0) + defer tc.Cleanup() + appState := tc.G.MobileAppState + l := NewLiveLocationTracker(globals.NewContext(tc.G, &globals.ChatContext{})) + ctx := context.Background() + coord := func(lat float64) chat1.Coordinate { return chat1.Coordinate{Lat: lat, Lon: 1} } + addTracker := func(msgID chat1.MessageID) *locationTrack { + track := newLocationTrack(chat1.ConversationID("conv"), msgID, time.Now().Add(time.Hour), false, 10, false) + l.Lock() + defer l.Unlock() + l.trackers[track.Key()] = track + return track + } + removeTracker := func(track *locationTrack) { + l.Lock() + defer l.Unlock() + l.removeTrackerLocked(ctx, track) + } + + lc := tc.G.MobileLifecycle + lifecycletest.ToBackground(lc) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State()) + l.LocationUpdate(ctx, coord(1)) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State(), "no trackers, no hold") + + first := addTracker(1) + second := addTracker(2) + l.LocationUpdate(ctx, coord(2)) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State()) + removeTracker(first) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State(), "still tracking") + removeTracker(second) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State()) + + // A fix in the foreground holds too, so backgrounding keeps the work running. + lc.UIActive() + third := addTracker(3) + l.LocationUpdate(ctx, coord(3)) + require.Equal(t, keybase1.MobileAppState_FOREGROUND, appState.State()) + lifecycletest.ToBackground(lc) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State()) + removeTracker(third) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State()) +} + +// WillTerminate is the one controller event that ends a live location hold. A +// fix after it opens a new one, rather than counting on the ended hold. +func TestLiveLocationTrackerHoldSurvivesWillTerminate(t *testing.T) { + t.Setenv("KEYBASE_APP_TYPE", string(libkb.MobileAppType)) + tc := libkb.SetupTest(t, "LiveLocationTrackerWillTerminate", 0) + defer tc.Cleanup() + appState := tc.G.MobileAppState + l := NewLiveLocationTracker(globals.NewContext(tc.G, &globals.ChatContext{})) + ctx := context.Background() + coord := func(lat float64) chat1.Coordinate { return chat1.Coordinate{Lat: lat, Lon: 1} } + + track := newLocationTrack(chat1.ConversationID("conv"), 1, time.Now().Add(time.Hour), false, 10, false) + l.Lock() + l.trackers[track.Key()] = track + l.Unlock() + + lc := tc.G.MobileLifecycle + lifecycletest.ToBackground(lc) + l.LocationUpdate(ctx, coord(1)) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State()) + + lc.WillTerminate(func() {}) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State(), "WillTerminate left a hold open") + + l.LocationUpdate(ctx, coord(2)) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State(), + "a fix after WillTerminate did not open a new hold") +} + +// A Start whose restored trackers are all gone (stopped, or none at all) +// still finds an outstanding hold from before the restore -- runRestoredLocked +// replaces the tracker map wholesale, so it never runs removeTrackerLocked for +// whatever was tracked previously. +func TestRestoredTrackersReleaseHoldWhenEmpty(t *testing.T) { + t.Setenv("KEYBASE_APP_TYPE", string(libkb.MobileAppType)) + tc := libkb.SetupTest(t, "LiveLocationRestoredReleasesHold", 0) + defer tc.Cleanup() + appState := tc.G.MobileAppState + l := NewLiveLocationTracker(globals.NewContext(tc.G, &globals.ChatContext{})) + ctx := context.Background() + + track := newLocationTrack(chat1.ConversationID("conv"), 1, time.Now().Add(time.Hour), false, 10, false) + l.Lock() + l.trackers[track.Key()] = track + l.Unlock() + + lc := tc.G.MobileLifecycle + lifecycletest.ToBackground(lc) + l.LocationUpdate(ctx, chat1.Coordinate{Lat: 1, Lon: 1}) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State(), "the fix opened a hold") + + stopped := newLocationTrack(chat1.ConversationID("conv"), 2, time.Now().Add(time.Hour), false, 10, true) + l.Lock() + l.runRestoredLocked([]*locationTrack{stopped}) + l.Unlock() + + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State(), + "a restore with nothing live left the old hold open") +} diff --git a/go/chat/maps/livelocation_throttle_test.go b/go/chat/maps/livelocation_throttle_test.go new file mode 100644 index 000000000000..1a5f653b4a48 --- /dev/null +++ b/go/chat/maps/livelocation_throttle_test.go @@ -0,0 +1,119 @@ +package maps + +import ( + "math" + "testing" + + "github.com/keybase/client/go/protocol/chat1" + "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/require" +) + +// north returns c moved due north by meters, which is exact under the +// spherical distance the throttle measures. +func north(c chat1.Coordinate, meters float64) chat1.Coordinate { + c.Lat += meters / earthRadiusMeters * 180 / math.Pi + return c +} + +func TestShouldRecordFix(t *testing.T) { + origin := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194, Accuracy: 10} + at := func(c chat1.Coordinate) *chat1.Coordinate { return &c } + + cases := []struct { + name string + state keybase1.MobileAppState + last fixThrottle + next chat1.Coordinate + record bool + after fixThrottle + }{ + { + name: "first fix since the watch started, in the background", + state: keybase1.MobileAppState_BACKGROUND, + next: origin, + record: true, + after: fixThrottle{prev: at(origin)}, + }, + { + name: "any move in the foreground", + state: keybase1.MobileAppState_FOREGROUND, + last: fixThrottle{prev: at(origin), pendingDistance: 3}, + next: north(origin, 1), + record: true, + after: fixThrottle{prev: at(north(origin, 1))}, + }, + { + name: "short move in the background", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{prev: at(origin)}, + next: north(origin, 10), + record: false, + after: fixThrottle{prev: at(north(origin, 10)), pendingDistance: 10}, + }, + { + name: "short move while background work runs", + state: keybase1.MobileAppState_BACKGROUNDACTIVE, + last: fixThrottle{prev: at(origin)}, + next: north(origin, 10), + record: false, + after: fixThrottle{prev: at(north(origin, 10)), pendingDistance: 10}, + }, + { + name: "short move while on screen but not active", + state: keybase1.MobileAppState_INACTIVE, + last: fixThrottle{prev: at(origin)}, + next: north(origin, 10), + record: false, + after: fixThrottle{prev: at(north(origin, 10)), pendingDistance: 10}, + }, + { + name: "long move in the background", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{prev: at(origin)}, + next: north(origin, 100), + record: true, + after: fixThrottle{prev: at(north(origin, 100))}, + }, + { + name: "unrecorded moves add up to the distance", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{prev: at(origin), pendingDistance: 60}, + next: north(origin, 10), + record: true, + after: fixThrottle{prev: at(north(origin, 10))}, + }, + { + name: "the distance is along the path, not from the last recorded fix", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{prev: at(north(origin, 40)), pendingDistance: 40}, + next: origin, + record: true, + after: fixThrottle{prev: at(origin)}, + }, + { + name: "exactly the distance", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{prev: at(origin), pendingDistance: 65}, + next: origin, + record: true, + after: fixThrottle{prev: at(origin)}, + }, + { + name: "just short of the distance", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{prev: at(origin), pendingDistance: 64.9}, + next: origin, + record: false, + after: fixThrottle{prev: at(origin), pendingDistance: 64.9}, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + record, after := shouldRecordFix(c.state, c.last, c.next) + require.Equal(t, c.record, record) + require.Equal(t, c.after.prev, after.prev) + require.InDelta(t, c.after.pendingDistance, after.pendingDistance, 1e-6) + }) + } +} diff --git a/go/chat/maps/livelocation_watch_test.go b/go/chat/maps/livelocation_watch_test.go new file mode 100644 index 000000000000..0b0c16aff966 --- /dev/null +++ b/go/chat/maps/livelocation_watch_test.go @@ -0,0 +1,343 @@ +package maps + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/keybase/client/go/chat/globals" + "github.com/keybase/client/go/chat/types" + "github.com/keybase/client/go/chat/utils" + "github.com/keybase/client/go/kbtest" + "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/libkb/lifecycle/lifecycletest" + "github.com/keybase/client/go/protocol/chat1" + "github.com/keybase/client/go/protocol/keybase1" + "github.com/keybase/clockwork" + "github.com/stretchr/testify/require" +) + +type fakeLocationWatcher struct { + sync.Mutex + calls []string +} + +func (w *fakeLocationWatcher) StartWatching() { w.record("start") } +func (w *fakeLocationWatcher) StopWatching() { w.record("stop") } + +func (w *fakeLocationWatcher) record(call string) { + w.Lock() + defer w.Unlock() + w.calls = append(w.calls, call) +} + +func (w *fakeLocationWatcher) Calls() []string { + w.Lock() + defer w.Unlock() + return append([]string(nil), w.calls...) +} + +type watchCall struct { + convID chat1.ConversationID + perm chat1.UIWatchPositionPerm +} + +type fakeWatchChatUI struct { + utils.NullChatUI + sync.Mutex + watches []watchCall + clears []chat1.LocationWatchID + nextID chat1.LocationWatchID +} + +func (u *fakeWatchChatUI) ChatWatchPosition(_ context.Context, convID chat1.ConversationID, + perm chat1.UIWatchPositionPerm, +) (chat1.LocationWatchID, error) { + u.Lock() + defer u.Unlock() + u.watches = append(u.watches, watchCall{convID: convID, perm: perm}) + u.nextID++ + return u.nextID, nil +} + +func (u *fakeWatchChatUI) ChatClearWatch(_ context.Context, id chat1.LocationWatchID) error { + u.Lock() + defer u.Unlock() + u.clears = append(u.clears, id) + return nil +} + +func (u *fakeWatchChatUI) Watches() []watchCall { + u.Lock() + defer u.Unlock() + return append([]watchCall(nil), u.watches...) +} + +func (u *fakeWatchChatUI) Clears() []chat1.LocationWatchID { + u.Lock() + defer u.Unlock() + return append([]chat1.LocationWatchID(nil), u.clears...) +} + +type nilCtxFactory struct{} + +func (nilCtxFactory) NewKeyFinder() types.KeyFinder { return nil } +func (nilCtxFactory) NewUPAKFinder() types.UPAKFinder { return nil } + +// newWatchTestTracker builds a tracker whose map unfurls fail right away (the +// mock chat helper returns no message), so the tracker loop runs without a +// chat server. chatUI nil means no UI is connected. +func newWatchTestTracker(t *testing.T, tc libkb.TestContext, watcher types.LocationWatcher, + chatUI libkb.ChatUI, +) *LiveLocationTracker { + tc.G.ChatHelper = kbtest.NewMockChatHelper() + tc.G.SetUIRouter(kbtest.NewMockUIRouter(chatUI)) + g := globals.NewContext(tc.G, &globals.ChatContext{ + CtxFactory: nilCtxFactory{}, + LocationWatcher: watcher, + }) + l := NewLiveLocationTracker(g) + l.SetClock(clockwork.NewFakeClock()) + t.Cleanup(func() { + l.StopAllTracking(context.Background()) + select { + case <-l.Stop(context.Background()): + case <-time.After(10 * time.Second): + t.Error("trackers did not stop") + } + }) + return l +} + +var watchTestConvID = chat1.ConversationID("conv") + +func startTestTracker(l *LiveLocationTracker, msgID chat1.MessageID) *locationTrack { + l.StartTracking(context.Background(), watchTestConvID, msgID, l.clock.Now().Add(time.Hour)) + l.Lock() + defer l.Unlock() + return l.trackers[newLocationTrack(watchTestConvID, msgID, time.Time{}, false, 0, false).Key()] +} + +func waitTrackerRemoved(t *testing.T, l *LiveLocationTracker, track *locationTrack) { + require.Eventually(t, func() bool { + l.Lock() + defer l.Unlock() + _, ok := l.trackers[track.Key()] + return !ok + }, 10*time.Second, 5*time.Millisecond) +} + +func TestLiveLocationTrackerNativeWatcher(t *testing.T) { + tc := libkb.SetupTest(t, "LiveLocationTrackerNativeWatcher", 0) + t.Cleanup(tc.Cleanup) + watcher := &fakeLocationWatcher{} + ui := &fakeWatchChatUI{} + l := newWatchTestTracker(t, tc, watcher, ui) + + first := startTestTracker(l, 1) + second := startTestTracker(l, 2) + require.Eventually(t, func() bool { return len(ui.Watches()) == 2 }, 10*time.Second, 5*time.Millisecond) + require.Equal(t, []string{"start"}, watcher.Calls(), "one native watch for both trackers") + for _, w := range ui.Watches() { + require.Equal(t, watchCall{convID: watchTestConvID, perm: chat1.UIWatchPositionPerm_ALWAYS}, w, + "the UI is still asked for permission") + } + + first.Stop() + waitTrackerRemoved(t, l, first) + require.Equal(t, []string{"start"}, watcher.Calls(), "still tracking") + + second.Stop() + waitTrackerRemoved(t, l, second) + require.Equal(t, []string{"start", "stop"}, watcher.Calls()) + require.Empty(t, ui.Clears(), "the UI never watched, so it never clears") + + third := startTestTracker(l, 3) + require.Eventually(t, func() bool { return len(watcher.Calls()) == 3 }, 10*time.Second, 5*time.Millisecond) + require.Equal(t, []string{"start", "stop", "start"}, watcher.Calls()) + third.Stop() + waitTrackerRemoved(t, l, third) + require.Equal(t, []string{"start", "stop", "start", "stop"}, watcher.Calls()) +} + +func TestLiveLocationTrackerChatUIWatch(t *testing.T) { + tc := libkb.SetupTest(t, "LiveLocationTrackerChatUIWatch", 0) + t.Cleanup(tc.Cleanup) + ui := &fakeWatchChatUI{} + l := newWatchTestTracker(t, tc, nil, ui) + + first := startTestTracker(l, 1) + second := startTestTracker(l, 2) + require.Eventually(t, func() bool { return len(ui.Watches()) == 2 }, 10*time.Second, 5*time.Millisecond) + + first.Stop() + waitTrackerRemoved(t, l, first) + require.Len(t, ui.Clears(), 1) + second.Stop() + waitTrackerRemoved(t, l, second) + require.ElementsMatch(t, []chat1.LocationWatchID{1, 2}, ui.Clears()) +} + +func TestLiveLocationTrackerRestoreStartsNativeWatch(t *testing.T) { + tc := libkb.SetupTest(t, "LiveLocationTrackerRestoreStartsNativeWatch", 0) + t.Cleanup(tc.Cleanup) + watcher := &fakeLocationWatcher{} + l := newWatchTestTracker(t, tc, watcher, nil) + + endTime := l.clock.Now().Add(time.Hour) + live := newLocationTrack(watchTestConvID, 1, endTime, false, 10, false) + other := newLocationTrack(watchTestConvID, 2, endTime, false, 10, false) + stopped := newLocationTrack(watchTestConvID, 3, endTime, false, 10, true) + l.Lock() + l.runRestoredLocked([]*locationTrack{live, other, stopped}) + l.Unlock() + + require.Eventually(t, func() bool { return len(watcher.Calls()) == 1 }, 10*time.Second, 5*time.Millisecond) + require.Equal(t, []string{"start"}, watcher.Calls()) + require.True(t, l.ActivelyTracking(context.Background())) + + live.Stop() + other.Stop() + waitTrackerRemoved(t, l, live) + waitTrackerRemoved(t, l, other) + require.Equal(t, []string{"start", "stop"}, watcher.Calls()) +} + +func TestLiveLocationTrackerNativeWatchStopsWhenTrackerEnds(t *testing.T) { + tc := libkb.SetupTest(t, "LiveLocationTrackerNativeWatchStopsWhenTrackerEnds", 0) + t.Cleanup(tc.Cleanup) + watcher := &fakeLocationWatcher{} + l := newWatchTestTracker(t, tc, watcher, nil) + clock := l.clock.(clockwork.FakeClock) + + track := startTestTracker(l, 1) + require.Eventually(t, func() bool { return len(watcher.Calls()) == 1 }, 10*time.Second, 5*time.Millisecond) + clock.BlockUntil(2) + clock.Advance(2 * time.Hour) + waitTrackerRemoved(t, l, track) + require.Equal(t, []string{"start", "stop"}, watcher.Calls()) +} + +type failingWatchChatUI struct { + utils.NullChatUI + attempts atomic.Int32 +} + +func (u *failingWatchChatUI) ChatWatchPosition(context.Context, chat1.ConversationID, + chat1.UIWatchPositionPerm, +) (chat1.LocationWatchID, error) { + u.attempts.Add(1) + return 0, errors.New("no UI yet") +} + +func TestLiveLocationTrackerChatUIWatchGivesUp(t *testing.T) { + tc := libkb.SetupTest(t, "LiveLocationTrackerChatUIWatchGivesUp", 0) + t.Cleanup(tc.Cleanup) + ui := &failingWatchChatUI{} + l := newWatchTestTracker(t, tc, nil, ui) + clock := l.clock.(clockwork.FakeClock) + + done := make(chan error, 1) + track := newLocationTrack(watchTestConvID, 1, clock.Now().Add(time.Hour), false, 10, false) + go func() { + _, err := l.startChatUIWatch(context.Background(), track) + done <- err + }() + // One try plus 21 retries, a second apart. + const maxAttempts = 22 + for n := int32(1); ; n++ { + require.Eventually(t, func() bool { return ui.attempts.Load() >= n }, 10*time.Second, time.Millisecond) + if n == maxAttempts { + break + } + clock.BlockUntil(1) + clock.Advance(time.Second) + } + select { + case err := <-done: + require.Error(t, err) + case <-time.After(10 * time.Second): + require.Fail(t, "still retrying", "after %d attempts", ui.attempts.Load()) + } + require.EqualValues(t, maxAttempts, ui.attempts.Load()) +} + +// A tracker whose watch never starts ends like any other: it leaves no tracker +// and no background-work hold, so the app can still reach BACKGROUND. +func TestLiveLocationTrackerFailedWatchLeavesNoHold(t *testing.T) { + t.Setenv("KEYBASE_APP_TYPE", string(libkb.MobileAppType)) + tc := libkb.SetupTest(t, "LiveLocationTrackerFailedWatchLeavesNoHold", 0) + t.Cleanup(tc.Cleanup) + ui := &failingWatchChatUI{} + l := newWatchTestTracker(t, tc, nil, ui) + clock := l.clock.(clockwork.FakeClock) + appState := tc.G.MobileAppState + + track := startTestTracker(l, 1) + require.NotNil(t, track) + // A fix while the watch is still retrying holds the app up. + require.Eventually(t, func() bool { return ui.attempts.Load() >= 1 }, 10*time.Second, time.Millisecond) + l.LocationUpdate(context.Background(), chat1.Coordinate{Lat: 1, Lon: 1}) + lifecycletest.ToBackground(tc.G.MobileLifecycle) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, appState.State()) + + for ui.attempts.Load() < 22 { + // Read the count while the retry is parked on the clock: Advance + // releases it, so a count read afterward can already include it. + clock.BlockUntil(1) + n := ui.attempts.Load() + clock.Advance(time.Second) + require.Eventually(t, func() bool { return ui.attempts.Load() > n }, 10*time.Second, time.Millisecond) + } + waitTrackerRemoved(t, l, track) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State()) + + // A later fix finds no tracker to hold the app up for. + tc.G.MobileLifecycle.UIActive() + l.LocationUpdate(context.Background(), chat1.Coordinate{Lat: 2, Lon: 2}) + lifecycletest.ToBackground(tc.G.MobileLifecycle) + require.Equal(t, keybase1.MobileAppState_BACKGROUND, appState.State()) +} + +// gatedClearChatUI holds the first watch's clear until gate closes. +type gatedClearChatUI struct { + fakeWatchChatUI + clearing chan struct{} + gate chan struct{} +} + +func (u *gatedClearChatUI) ChatClearWatch(ctx context.Context, id chat1.LocationWatchID) error { + if id == 1 { + close(u.clearing) + <-u.gate + } + return u.fakeWatchChatUI.ChatClearWatch(ctx, id) +} + +// Stop waits for the trackers it stops, not for one started after it. +func TestLiveLocationTrackerStopWaitsOnlyForItsTrackers(t *testing.T) { + tc := libkb.SetupTest(t, "LiveLocationTrackerStopWaitsOnlyForItsTrackers", 0) + t.Cleanup(tc.Cleanup) + ui := &gatedClearChatUI{clearing: make(chan struct{}), gate: make(chan struct{})} + l := newWatchTestTracker(t, tc, nil, ui) + + require.NotNil(t, startTestTracker(l, 1)) + require.Eventually(t, func() bool { return len(ui.Watches()) == 1 }, 10*time.Second, 5*time.Millisecond) + stopped := l.Stop(context.Background()) + select { + case <-ui.clearing: + case <-time.After(10 * time.Second): + require.FailNow(t, "the stopped tracker did not exit") + } + // The stopped tracker is still exiting when the next one starts. + require.NotNil(t, startTestTracker(l, 2)) + close(ui.gate) + select { + case <-stopped: + case <-time.After(10 * time.Second): + require.FailNow(t, "Stop waited for a tracker started after it") + } +} diff --git a/go/chat/types/interfaces.go b/go/chat/types/interfaces.go index 3fc7a7da8d6e..2ff29723e0ee 100644 --- a/go/chat/types/interfaces.go +++ b/go/chat/types/interfaces.go @@ -464,6 +464,15 @@ type ShareIntentDonator interface { DeleteDonation(conversationID string) } +// LocationWatcher runs the OS location service natively (iOS), so live +// location keeps working without the UI. Fixes come back through +// LiveLocationTracker.NativeLocationUpdate. When nil, the chat UI watches +// position. +type LocationWatcher interface { + StartWatching() + StopWatching() +} + type StellarLoader interface { LoadPayment(ctx context.Context, convID chat1.ConversationID, msgID chat1.MessageID, senderUsername string, paymentID stellar1.PaymentID) *chat1.UIPaymentInfo LoadRequest(ctx context.Context, convID chat1.ConversationID, msgID chat1.MessageID, senderUsername string, requestID stellar1.KeybaseRequestID) *chat1.UIRequestInfo @@ -577,6 +586,7 @@ type LiveLocationTracker interface { GetCurrentPosition(ctx context.Context, convID chat1.ConversationID, msgID chat1.MessageID) StartTracking(ctx context.Context, convID chat1.ConversationID, msgID chat1.MessageID, endTime time.Time) LocationUpdate(ctx context.Context, coord chat1.Coordinate) + NativeLocationUpdate(ctx context.Context, coord chat1.Coordinate) GetCoordinates(ctx context.Context, key LiveLocationKey) []chat1.Coordinate GetEndTime(ctx context.Context, key LiveLocationKey) *time.Time ActivelyTracking(ctx context.Context) bool diff --git a/go/chat/unfurl/scraper_test.go b/go/chat/unfurl/scraper_test.go index 6a077e9b2700..f97b8e823fd2 100644 --- a/go/chat/unfurl/scraper_test.go +++ b/go/chat/unfurl/scraper_test.go @@ -387,6 +387,10 @@ func (t *testingLiveLocationTracker) LocationUpdate(ctx context.Context, coord c t.coords = append(t.coords, coord) } +func (t *testingLiveLocationTracker) NativeLocationUpdate(ctx context.Context, coord chat1.Coordinate) { + t.LocationUpdate(ctx, coord) +} + func (t *testingLiveLocationTracker) GetCoordinates(ctx context.Context, key types.LiveLocationKey) []chat1.Coordinate { return t.coords } From 3fa829a0c8e8df40983a2dcc6b62c8dc65ba7e6a Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 13:51:51 -0400 Subject: [PATCH 2/7] feat(ios): run the OS location service from native for live location Init and InitOnce take a NativeLocationWatcher, which iOS supplies and Android passes nil for -- Android keeps the expo background location task. LocationWatcher.swift owns a CLLocationManager with the same options expo used, started and stopped by Go and reporting every fix back through LocationUpdate off the main thread. It is built in didFinishLaunching, before Go restores its trackers, so an app relaunched by significant-change monitoring picks the watch back up. LocationUpdate drops fixes taken before Init finishes or while logged out, since there is no tracker to take them. --- go/bind/keybase.go | 29 +++- go/bind/location_test.go | 162 ++++++++++++++++++ .../java/io/keybase/ossifrage/MainActivity.kt | 2 +- shared/ios/Keybase.xcodeproj/project.pbxproj | 4 + shared/ios/Keybase/AppDelegate.swift | 5 +- shared/ios/Keybase/LocationWatcher.swift | 88 ++++++++++ 6 files changed, 285 insertions(+), 5 deletions(-) create mode 100644 go/bind/location_test.go create mode 100644 shared/ios/Keybase/LocationWatcher.swift diff --git a/go/bind/keybase.go b/go/bind/keybase.go index 3c8aece0fa7c..3c4c16e010d3 100644 --- a/go/bind/keybase.go +++ b/go/bind/keybase.go @@ -186,6 +186,15 @@ type ShareIntentDonator interface { DeleteDonation(conversationID string) } +// NativeLocationWatcher is implemented by the native iOS layer. It runs the OS +// location service while live location is on and reports each fix through +// LocationUpdate, so live location works without JS. When nil (Android, +// desktop), the chat UI watches position instead. +type NativeLocationWatcher interface { + StartWatching() + StopWatching() +} + // shareIntentDonatorAdapter adapts keybase.ShareIntentDonator to types.ShareIntentDonator. type shareIntentDonatorAdapter struct { wrapped ShareIntentDonator @@ -325,10 +334,10 @@ func setInited() { func InitOnce(homeDir, mobileSharedHome, logFile, runModeStr string, accessGroupOverride bool, dnsNSFetcher ExternalDNSNSFetcher, nvh NativeVideoHelper, mobileOsVersion string, isIPad bool, installReferrerListener NativeInstallReferrerListener, isIOS bool, - shareIntentDonator ShareIntentDonator, + shareIntentDonator ShareIntentDonator, locationWatcher NativeLocationWatcher, ) { startOnce.Do(func() { - if err := Init(homeDir, mobileSharedHome, logFile, runModeStr, accessGroupOverride, dnsNSFetcher, nvh, mobileOsVersion, isIPad, installReferrerListener, isIOS, shareIntentDonator); err != nil { + if err := Init(homeDir, mobileSharedHome, logFile, runModeStr, accessGroupOverride, dnsNSFetcher, nvh, mobileOsVersion, isIPad, installReferrerListener, isIOS, shareIntentDonator, locationWatcher); err != nil { log("Init error: %s", err) } }) @@ -338,7 +347,7 @@ func InitOnce(homeDir, mobileSharedHome, logFile, runModeStr string, func Init(homeDir, mobileSharedHome, logFile, runModeStr string, accessGroupOverride bool, externalDNSNSFetcher ExternalDNSNSFetcher, nvh NativeVideoHelper, mobileOsVersion string, isIPad bool, installReferrerListener NativeInstallReferrerListener, isIOS bool, - shareIntentDonator ShareIntentDonator, + shareIntentDonator ShareIntentDonator, locationWatcher NativeLocationWatcher, ) (err error) { // Dump all goroutines on a fatal error; the GOTRACEBACK env var can't be // used here since the runtime reads it before Init runs. @@ -462,6 +471,7 @@ func Init(homeDir, mobileSharedHome, logFile, runModeStr string, if shareIntentDonator != nil { kbChatCtx.ShareIntentDonator = shareIntentDonatorAdapter{wrapped: shareIntentDonator} } + kbChatCtx.LocationWatcher = locationWatcher // Runs the startup login attempt and then the long-lived background // tasks. Off the Init thread so a slow login can't hold up app launch; // must start after the chat context fields above are set since chat @@ -896,6 +906,19 @@ func AppUIInactive() { kbCtx.MobileLifecycle.UIInactive() } +// LocationUpdate reports every location fix from the native location service; +// the tracker decides which ones to record. +func LocationUpdate(lat, lon float64, accuracy int) { + if !isInited() || !kbCtx.ActiveDevice.HaveKeys() { + return + } + locationUpdate(kbChatCtx.LiveLocationTracker, lat, lon, accuracy) +} + +func locationUpdate(tracker types.LiveLocationTracker, lat, lon float64, accuracy int) { + tracker.NativeLocationUpdate(context.Background(), chat1.Coordinate{Lat: lat, Lon: lon, Accuracy: float64(accuracy)}) +} + func waitForInit(maxDur time.Duration) error { if isInited() { return nil diff --git a/go/bind/location_test.go b/go/bind/location_test.go new file mode 100644 index 000000000000..a1c1f600b2f8 --- /dev/null +++ b/go/bind/location_test.go @@ -0,0 +1,162 @@ +package keybase + +import ( + "context" + "encoding/base64" + "fmt" + "sync" + "testing" + "time" + + "github.com/keybase/client/go/chat/globals" + "github.com/keybase/client/go/chat/maps" + "github.com/keybase/client/go/chat/types" + "github.com/keybase/client/go/kbtest" + "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/libkb/lifecycle/lifecycletest" + "github.com/keybase/client/go/protocol/chat1" + "github.com/keybase/client/go/protocol/keybase1" + "github.com/keybase/clockwork" + "github.com/stretchr/testify/require" +) + +type countingLocationWatcher struct{ starts, stops chan struct{} } + +func (w countingLocationWatcher) StartWatching() { w.starts <- struct{}{} } +func (w countingLocationWatcher) StopWatching() { w.stops <- struct{}{} } + +type nilCtxFactory struct{} + +func (nilCtxFactory) NewKeyFinder() types.KeyFinder { return nil } +func (nilCtxFactory) NewUPAKFinder() types.UPAKFinder { return nil } + +func TestLocationUpdateReachesTrackers(t *testing.T) { + t.Setenv("KEYBASE_APP_TYPE", string(libkb.MobileAppType)) + tc := libkb.SetupTest(t, "LocationUpdateReachesTrackers", 0) + defer tc.Cleanup() + tc.G.ChatHelper = kbtest.NewMockChatHelper() + tc.G.SetUIRouter(kbtest.NewMockUIRouter(nil)) + watcher := countingLocationWatcher{starts: make(chan struct{}, 10), stops: make(chan struct{}, 10)} + var nativeWatcher NativeLocationWatcher = watcher + g := globals.NewContext(tc.G, &globals.ChatContext{CtxFactory: nilCtxFactory{}, LocationWatcher: nativeWatcher}) + tracker := maps.NewLiveLocationTracker(g) + clock := clockwork.NewFakeClock() + tracker.SetClock(clock) + ctx := context.Background() + lifecycletest.ToBackground(tc.G.MobileLifecycle) + + startTracking := func(msgID chat1.MessageID) types.LiveLocationKey { + tracker.StartTracking(ctx, chat1.ConversationID("conv"), msgID, clock.Now().Add(time.Hour)) + select { + case <-watcher.starts: + case <-time.After(10 * time.Second): + require.Fail(t, "native watch never started") + } + return types.LiveLocationKey(base64.StdEncoding.EncodeToString( + fmt.Appendf(nil, "%s:%d", chat1.ConversationID("conv"), msgID))) + } + stopTracking := func() { + tracker.StopAllTracking(ctx) + select { + case <-tracker.Stop(ctx): + case <-time.After(10 * time.Second): + require.Fail(t, "tracker did not stop") + } + select { + case <-watcher.stops: + default: + require.Fail(t, "native watch never stopped") + } + require.Equal(t, keybase1.MobileAppState_BACKGROUND, tc.G.MobileAppState.State()) + } + fix := func(lat float64) chat1.Coordinate { return chat1.Coordinate{Lat: lat, Lon: -73.25, Accuracy: 12} } + // waitRecorded waits for the tracker at key to take the fix at lat, and + // returns its coordinates. The tracker also takes the last coordinate it + // has when it starts, which can repeat the first fix; repeats are dropped. + waitRecorded := func(key types.LiveLocationKey, lat float64) (res []chat1.Coordinate) { + require.Eventually(t, func() bool { + coords := tracker.GetCoordinates(ctx, key) + return coords[len(coords)-1] == fix(lat) + }, 10*time.Second, time.Millisecond, "coordinate never reached the tracker") + for _, c := range tracker.GetCoordinates(ctx, key) { + if len(res) == 0 || res[len(res)-1] != c { + res = append(res, c) + } + } + return res + } + + key := startTracking(1) + // The first fix is recorded even in the background. + locationUpdate(tracker, 40.5, -73.25, 12) + require.Equal(t, []chat1.Coordinate{fix(40.5)}, waitRecorded(key, 40.5)) + require.Equal(t, keybase1.MobileAppState_BACKGROUNDACTIVE, tc.G.MobileAppState.State()) + // About 11m north, too short a move to record in the background, then + // about 111m further. The coordinates arrive in order, so once the last one + // is in, the short move would be too. + locationUpdate(tracker, 40.5001, -73.25, 12) + locationUpdate(tracker, 40.5011, -73.25, 12) + require.Equal(t, []chat1.Coordinate{fix(40.5), fix(40.5011)}, waitRecorded(key, 40.5011)) + stopTracking() + + // A new watch records its first fix however short the move. + key = startTracking(2) + locationUpdate(tracker, 40.5012, -73.25, 12) + waitRecorded(key, 40.5012) + stopTracking() +} + +type recordingLiveLocationTracker struct { + types.LiveLocationTracker + sync.Mutex + coords []chat1.Coordinate +} + +func (r *recordingLiveLocationTracker) NativeLocationUpdate(_ context.Context, coord chat1.Coordinate) { + r.Lock() + defer r.Unlock() + r.coords = append(r.coords, coord) +} + +func (r *recordingLiveLocationTracker) Coords() []chat1.Coordinate { + r.Lock() + defer r.Unlock() + return append([]chat1.Coordinate(nil), r.coords...) +} + +func TestLocationUpdateGuards(t *testing.T) { + resetConnStateForTest(t) + savedChatCtx := kbChatCtx + t.Cleanup(func() { kbChatCtx = savedChatCtx }) + setInitComplete := func(v bool) { + initMutex.Lock() + defer initMutex.Unlock() + initComplete = v + } + + tc := libkb.SetupTest(t, "LocationUpdateGuards", 0) + defer tc.Cleanup() + tracker := &recordingLiveLocationTracker{} + kbCtx = tc.G + kbChatCtx = &globals.ChatContext{LiveLocationTracker: tracker} + + setInitComplete(true) + LocationUpdate(1, 2, 3) + require.Empty(t, tracker.Coords(), "dropped while logged out") + + sigKey, err := libkb.GenerateNaclSigningKeyPair() + require.NoError(t, err) + encKey, err := libkb.GenerateNaclDHKeyPair() + require.NoError(t, err) + uv := keybase1.UserVersion{Uid: keybase1.MakeTestUID(1), EldestSeqno: 1} + require.NoError(t, tc.G.ActiveDevice.Set(libkb.NewMetaContextForTest(tc), uv, keybase1.DeviceID("dev"), + sigKey, encKey, "testuser-device", 0, libkb.KeychainModeNone)) + + setInitComplete(false) + LocationUpdate(1, 2, 3) + require.Empty(t, tracker.Coords(), "dropped before Init completes") + + setInitComplete(true) + LocationUpdate(1, 2, 3) + require.Equal(t, []chat1.Coordinate{{Lat: 1, Lon: 2, Accuracy: 3}}, tracker.Coords()) +} diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt index f1aaabaf42b4..96e85fae4b72 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt @@ -422,7 +422,7 @@ class MainActivity : ReactActivity() { val isIPad = false val isIOS = false Keybase.initOnce(context.filesDir.path, "", context.getFileStreamPath("service.log").absolutePath, "prod", false, - DNSNSFetcher(), VideoHelper(), mobileOsVersion, isIPad, KBInstallReferrerListener(context), isIOS, null) + DNSNSFetcher(), VideoHelper(), mobileOsVersion, isIPad, KBInstallReferrerListener(context), isIOS, null, null) } } } diff --git a/shared/ios/Keybase.xcodeproj/project.pbxproj b/shared/ios/Keybase.xcodeproj/project.pbxproj index 04efe02c9f9d..5530d8122d57 100644 --- a/shared/ios/Keybase.xcodeproj/project.pbxproj +++ b/shared/ios/Keybase.xcodeproj/project.pbxproj @@ -41,6 +41,7 @@ DBDCF30E1B8D03DD00BA95D8 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = DBDCF3081B8D03DD00BA95D8 /* Images.xcassets */; }; DBDF89F62DF7779900EA18C2 /* Pusher.swift in Sources */ = {isa = PBXBuildFile; fileRef = DBDF89F52DF7779900EA18C2 /* Pusher.swift */; }; DBF123462DF1234500A12345 /* ShareIntentDonatorImpl.swift in Sources */ = {isa = PBXBuildFile; fileRef = DBF123452DF1234500A12345 /* ShareIntentDonatorImpl.swift */; }; + DBLOCWATCH00270000000002 /* LocationWatcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = DBLOCWATCH00270000000001 /* LocationWatcher.swift */; }; DBPERF022600000002 /* PerfFPSMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = DBPERF022600000001 /* PerfFPSMonitor.swift */; }; DBSCENE00270000000000002 /* SceneDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = DBSCENE00270000000000001 /* SceneDelegate.swift */; }; /* End PBXBuildFile section */ @@ -120,6 +121,7 @@ DBDCF3441B8D04FC00BA95D8 /* Keybase-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "Keybase-Bridging-Header.h"; sourceTree = ""; }; DBDF89F52DF7779900EA18C2 /* Pusher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Pusher.swift; sourceTree = ""; }; DBF123452DF1234500A12345 /* ShareIntentDonatorImpl.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareIntentDonatorImpl.swift; sourceTree = ""; }; + DBLOCWATCH00270000000001 /* LocationWatcher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationWatcher.swift; sourceTree = ""; }; DBPERF022600000001 /* PerfFPSMonitor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PerfFPSMonitor.swift; sourceTree = ""; }; DBSCENE00270000000000001 /* SceneDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SceneDelegate.swift; sourceTree = ""; }; F68DC40B579A1F9AC0F34950 /* Pods_KeybaseShare.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_KeybaseShare.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -281,6 +283,7 @@ DBD252972DF32D5C008A43FF /* Fs.swift */, DBDF89F52DF7779900EA18C2 /* Pusher.swift */, DBF123452DF1234500A12345 /* ShareIntentDonatorImpl.swift */, + DBLOCWATCH00270000000001 /* LocationWatcher.swift */, DBPERF022600000001 /* PerfFPSMonitor.swift */, 832341AE1AAA6A7D00B99B32 /* Libraries */, DAA243C38335068656BC36B1 /* PrivacyInfo.xcprivacy */, @@ -599,6 +602,7 @@ DB07050522E21B8B002F273D /* KeepThisFile.swift in Sources */, DBDF89F62DF7779900EA18C2 /* Pusher.swift in Sources */, DBF123462DF1234500A12345 /* ShareIntentDonatorImpl.swift in Sources */, + DBLOCWATCH00270000000002 /* LocationWatcher.swift in Sources */, DBD252982DF32D5C008A43FF /* Fs.swift in Sources */, DBPERF022600000002 /* PerfFPSMonitor.swift in Sources */, 9E1460E4A90E8D73A7347FED /* ExpoModulesProvider.swift in Sources */, diff --git a/shared/ios/Keybase/AppDelegate.swift b/shared/ios/Keybase/AppDelegate.swift index 42cac6f2eeae..08603d0d7a33 100644 --- a/shared/ios/Keybase/AppDelegate.swift +++ b/shared/ios/Keybase/AppDelegate.swift @@ -22,6 +22,7 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi var resignImageView: UIImageView? var fsPaths: [String: String] = [:] private let lifecycle = AppLifecycleForwarder() + private var locationWatcher: LocationWatcher? var iph: ItemProviderHelper? private var startupLogFileHandle: FileHandle? private let logQueue = DispatchQueue(label: "kb.startup.log", qos: .utility) @@ -183,7 +184,9 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi log.info("Starting KeybaseInit (synchronous)...") var err: NSError? let shareIntentDonator = ShareIntentDonatorImpl() - Keybasego.KeybaseInit(self.fsPaths["homedir"], self.fsPaths["sharedHome"], self.fsPaths["logFile"], "prod", securityAccessGroupOverride, nil, nil, systemVer, isIPad, nil, isIOS, shareIntentDonator, &err) + let locationWatcher = LocationWatcher() + self.locationWatcher = locationWatcher + Keybasego.KeybaseInit(self.fsPaths["homedir"], self.fsPaths["sharedHome"], self.fsPaths["logFile"], "prod", securityAccessGroupOverride, nil, nil, systemVer, isIPad, nil, isIOS, shareIntentDonator, locationWatcher, &err) if let err { let initResult = "FAILED: \(err.localizedDescription) (code=\(err.code) domain=\(err.domain))" log.error("KeybaseInit FAILED: \(err.localizedDescription, privacy: .public)") diff --git a/shared/ios/Keybase/LocationWatcher.swift b/shared/ios/Keybase/LocationWatcher.swift new file mode 100644 index 000000000000..6db76692af97 --- /dev/null +++ b/shared/ios/Keybase/LocationWatcher.swift @@ -0,0 +1,88 @@ +import CoreLocation +import Keybasego +import os + +private let log = Logger(subsystem: "com.keybase.app", category: "location") + +// Runs the OS location service for live location (go/chat/maps) without JS. Go +// starts and stops watching; every fix goes back to Go, which decides which to +// record. Created in didFinishLaunching, before Go restores its trackers, so an +// app relaunched by significant-change monitoring starts watching again. Its +// CLLocationManager options match expo-location's background task, which +// Android still uses. +final class LocationWatcher: NSObject, Keybasego.KeybaseNativeLocationWatcherProtocol, CLLocationManagerDelegate { + // Everything below is main thread only. + private let manager = CLLocationManager() + private var wanted = false + private var running = false + + // Go records a fix to disk, so fixes go to it off the main thread, in order. + private let goQueue = DispatchQueue(label: "com.keybase.app.location", qos: .utility) + + override init() { + super.init() + manager.delegate = self + } + + // Called by Go on a Go thread. + func startWatching() { + DispatchQueue.main.async { + self.wanted = true + self.apply() + } + } + + func stopWatching() { + DispatchQueue.main.async { + self.wanted = false + self.apply() + } + } + + // The prompt is asked for in JS when sharing starts; once the user answers, + // this starts watching if Go still wants it. + func locationManagerDidChangeAuthorization(_ manager: CLLocationManager) { + apply() + } + + func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) { + guard running else { return } + let fixes = locations.filter { $0.horizontalAccuracy >= 0 }.map { + (coordinate: $0.coordinate, accuracy: Int($0.horizontalAccuracy)) + } + goQueue.async { + for fix in fixes { + Keybasego.KeybaseLocationUpdate(fix.coordinate.latitude, fix.coordinate.longitude, fix.accuracy) + } + } + } + + func locationManager(_ manager: CLLocationManager, didFailWithError error: Error) { + log.error("location update failed: \(error.localizedDescription, privacy: .public)") + } + + private func apply() { + let status = manager.authorizationStatus + let authorized = status == .authorizedAlways || status == .authorizedWhenInUse + if wanted && !authorized { + log.warning("not watching location: not authorized (status \(status.rawValue))") + } + if wanted && authorized && !running { + log.info("starting location updates") + running = true + manager.allowsBackgroundLocationUpdates = true + manager.desiredAccuracy = kCLLocationAccuracyHundredMeters + manager.distanceFilter = kCLDistanceFilterNone + manager.activityType = .other + manager.pausesLocationUpdatesAutomatically = true + manager.showsBackgroundLocationIndicator = true + manager.startUpdatingLocation() + manager.startMonitoringSignificantLocationChanges() + } else if running && !(wanted && authorized) { + log.info("stopping location updates") + running = false + manager.stopUpdatingLocation() + manager.stopMonitoringSignificantLocationChanges() + } + } +} From 9f0a32522b6b68871da09a7d3f690075a97bf11b Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 13:51:56 -0400 Subject: [PATCH 3/7] fix(js): iOS leaves location to native and drops the legacy expo task onChatWatchPosition still asks for the permission the native watcher can't prompt for, then stops: on iOS the OS watch is Go's now, and running the expo background location task alongside it would mean two CLLocationManagers reporting the same fixes. Android is unchanged. Builds from before this left the expo task registered, and expo restores it into a second manager on every launch, so JS unregisters it once at startup. It is early enough to matter: with no UMAppLoader registered, expo can never start JS for a restored task on a background launch. --- shared/constants/init/index.tsx | 23 ++++ shared/constants/init/location-watch.test.ts | 129 +++++++++++++++++++ shared/constants/init/platform-types.ts | 2 + 3 files changed, 154 insertions(+) create mode 100644 shared/constants/init/location-watch.test.ts diff --git a/shared/constants/init/index.tsx b/shared/constants/init/index.tsx index 985cf814046c..505e7c24b795 100644 --- a/shared/constants/init/index.tsx +++ b/shared/constants/init/index.tsx @@ -66,6 +66,23 @@ const ensureBackgroundTask = (ExpoTaskManager: ExpoTaskManagerModule) => { }) } +// Builds from before native iOS location left this expo task registered, and expo restores it +// into a second CLLocationManager on every launch. JS is early enough to remove it: with no +// UMAppLoader registered, expo can never start JS for a restored task on a background launch +// (expo-task-manager EXTaskService.m `_loadAppWithId:appUrl:`). +export const unregisterLegacyIOSLocationTask = async () => { + if (!isIOS) return + const {ExpoTaskManager} = _getNative() + try { + if (await ExpoTaskManager.isTaskRegisteredAsync(locationTaskName)) { + await ExpoTaskManager.unregisterTaskAsync(locationTaskName) + logger.info('[location] removed the legacy iOS background location task') + } + } catch (error) { + logger.info('[location] failed to remove the legacy iOS background location task: ' + String(error)) + } +} + const setPermissionDeniedCommandStatus = (conversationIDKey: T.Chat.ConversationIDKey, text: string) => { setThreadInputCommandStatus(conversationIDKey, { actions: [T.RPCChat.UICommandStatusActionTyp.appsettings], @@ -92,6 +109,9 @@ const onChatWatchPosition = async ( ) } + // iOS watches location natively (ios/Keybase/LocationWatcher.swift), so JS only asks for permission. + if (isIOS) return + locationRefs++ if (locationRefs === 1) { @@ -112,6 +132,7 @@ const onChatWatchPosition = async ( } const onChatClearWatch = async () => { + if (isIOS) return const {ExpoLocation, ExpoTaskManager} = _getNative() locationRefs-- if (locationRefs <= 0) { @@ -452,6 +473,8 @@ const _initNativePlatformListener = () => { initPushListener() + ignorePromise(unregisterLegacyIOSLocationTask()) + const {NetInfo} = _getNative() NetInfo.addEventListener(({type}) => { useShellState.getState().dispatch.osNetworkStatusChanged(type !== NetInfo.NetInfoStateType.none, type) diff --git a/shared/constants/init/location-watch.test.ts b/shared/constants/init/location-watch.test.ts new file mode 100644 index 000000000000..55868d1fd195 --- /dev/null +++ b/shared/constants/init/location-watch.test.ts @@ -0,0 +1,129 @@ +/// +import type * as Init from './index' +import type * as EngineGen from '@/constants/rpc' + +// The init module picks its mobile behavior from the platform globals, so each test loads it +// fresh with them set and the native modules mocked. + +const calls = new Array() +const originalGlobals = {isAndroid: global.isAndroid, isIOS: global.isIOS, isMobile: global.isMobile} + +const load = (platform: 'ios' | 'android'): typeof Init => { + global.isMobile = true + global.isIOS = platform === 'ios' + global.isAndroid = platform === 'android' + jest.resetModules() + jest.doMock('./platform', () => ({ + getNative: () => ({ + ExpoLocation: { + startLocationUpdatesAsync: async () => { + calls.push('startLocationUpdates') + return Promise.resolve() + }, + stopLocationUpdatesAsync: async () => { + calls.push('stopLocationUpdates') + return Promise.resolve() + }, + }, + ExpoTaskManager: { + defineTask: () => { + calls.push('defineTask') + }, + // Registered until it is unregistered, like the real task store. + isTaskRegisteredAsync: async () => { + calls.push('isTaskRegistered') + return Promise.resolve(!calls.includes('unregisterTask')) + }, + unregisterTaskAsync: async () => { + calls.push('unregisterTask') + return Promise.resolve() + }, + }, + requestLocationPermission: async (perm: unknown) => { + calls.push(`requestPermission:${String(perm)}`) + return Promise.resolve() + }, + }), + })) + jest.doMock('./shared', () => ({ + _onEngineIncoming: () => {}, + })) + // pulls in the mobile theme, which needs more of react-native than the test mock has + jest.doMock('@/fs/common/lifecycle', () => ({})) + return require('./index') as typeof Init +} + +const watchPosition = () => + ({ + payload: { + params: {convID: new Uint8Array([0xaa, 0xbb]), perm: 1}, + response: { + result: () => { + calls.push('result') + }, + }, + }, + type: 'chat.1.chatUi.chatWatchPosition', + }) as unknown as EngineGen.Actions + +const clearWatch = () => + ({ + payload: {params: {id: 1}, response: {result: () => {}}}, + type: 'chat.1.chatUi.chatClearWatch', + }) as unknown as EngineGen.Actions + +const flush = async () => new Promise(resolve => setTimeout(resolve, 0)) + +afterEach(() => { + calls.length = 0 + jest.dontMock('./platform') + jest.dontMock('./shared') + jest.dontMock('@/fs/common/lifecycle') + jest.resetModules() + global.isMobile = originalGlobals.isMobile + global.isIOS = originalGlobals.isIOS + global.isAndroid = originalGlobals.isAndroid +}) + +test('iOS only asks for permission; the native watcher runs location', async () => { + const init = load('ios') + init.onEngineIncoming(watchPosition()) + await flush() + init.onEngineIncoming(clearWatch()) + await flush() + + expect(calls).toEqual(['result', 'requestPermission:1']) +}) + +test('Android asks for permission and runs the expo location task', async () => { + const init = load('android') + init.onEngineIncoming(watchPosition()) + await flush() + init.onEngineIncoming(clearWatch()) + await flush() + + expect(calls).toEqual([ + 'result', + 'requestPermission:1', + 'defineTask', + 'startLocationUpdates', + 'stopLocationUpdates', + ]) +}) + +// The second run stands for every launch after the cleanup: unregistering a task that is gone +// throws E_TASK_NOT_FOUND, so it must not be asked for again. +test('iOS removes the legacy expo background location task once', async () => { + const init = load('ios') + await init.unregisterLegacyIOSLocationTask() + await init.unregisterLegacyIOSLocationTask() + + expect(calls).toEqual(['isTaskRegistered', 'unregisterTask', 'isTaskRegistered']) +}) + +test('Android keeps its expo background location task', async () => { + const init = load('android') + await init.unregisterLegacyIOSLocationTask() + + expect(calls).toEqual([]) +}) diff --git a/shared/constants/init/platform-types.ts b/shared/constants/init/platform-types.ts index 8b44511da42a..8831418f3305 100644 --- a/shared/constants/init/platform-types.ts +++ b/shared/constants/init/platform-types.ts @@ -14,6 +14,8 @@ export type NetInfoModule = { } export type ExpoTaskManagerModule = { defineTask: (taskName: string, cb: (params: {data: unknown; error: unknown}) => Promise) => void + isTaskRegisteredAsync: (taskName: string) => Promise + unregisterTaskAsync: (taskName: string) => Promise } export type DesktopModules = { From 9826b793d73e4ebf1607602ae75abdec58826790 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:33:00 -0400 Subject: [PATCH 4/7] fix(chat): throttle background fixes by displacement, not path length The background throttle summed fix-to-fix distance and recorded a fix once that sum reached 65m. The iOS watcher asks for hundred-meter accuracy with no distance filter, so a phone sitting still reports fixes that jitter by tens of meters, and their sum passed 65m within a few fixes. Each one was recorded, saved and posted as a move that never happened, and kept the device doing that work while it sat still. Measure the straight-line distance from the last recorded fix instead. Jitter around one spot stays inside the radius, while a slow drift still builds displacement and gets recorded once it covers the distance. --- go/chat/maps/livelocation.go | 25 +++---- go/chat/maps/livelocation_throttle_test.go | 85 ++++++++++++---------- 2 files changed, 58 insertions(+), 52 deletions(-) diff --git a/go/chat/maps/livelocation.go b/go/chat/maps/livelocation.go index 5cb358d1a153..a7ae7f04d665 100644 --- a/go/chat/maps/livelocation.go +++ b/go/chat/maps/livelocation.go @@ -478,27 +478,22 @@ const earthRadiusMeters = 6371008.8 // fixThrottle is what shouldRecordFix knows of the fixes since the native // watch started. type fixThrottle struct { - // prev is the latest fix, recorded or not; nil until the first one. - prev *chat1.Coordinate - // pendingDistance is how far the device has moved, fix to fix, since the - // last recorded fix. - pendingDistance float64 + // lastRecorded is the latest recorded fix; nil until the first one. + lastRecorded *chat1.Coordinate } // shouldRecordFix decides whether a native fix gets recorded, and returns the // throttle to use for the next one. Out of the foreground a fix is recorded -// only once the device has moved backgroundFixDistance since the last one -// recorded. The first fix after the watch starts is recorded right away, so the -// move that relaunched the app gets posted. +// only once it lies backgroundFixDistance in a straight line from the last one +// recorded, so a stationary device's GPS jitter, which wanders back and forth +// around one spot, never adds up to a move. The first fix after the watch +// starts is recorded right away, so the move that relaunched the app gets +// posted. func shouldRecordFix(state keybase1.MobileAppState, last fixThrottle, next chat1.Coordinate) (bool, fixThrottle) { - if last.prev != nil { - last.pendingDistance += distanceMeters(*last.prev, next) - } - record := last.prev == nil || state == keybase1.MobileAppState_FOREGROUND || - last.pendingDistance >= backgroundFixDistance - last.prev = &next + record := last.lastRecorded == nil || state == keybase1.MobileAppState_FOREGROUND || + distanceMeters(*last.lastRecorded, next) >= backgroundFixDistance if record { - last.pendingDistance = 0 + last.lastRecorded = &next } return record, last } diff --git a/go/chat/maps/livelocation_throttle_test.go b/go/chat/maps/livelocation_throttle_test.go index 1a5f653b4a48..0741ad69dc37 100644 --- a/go/chat/maps/livelocation_throttle_test.go +++ b/go/chat/maps/livelocation_throttle_test.go @@ -26,94 +26,105 @@ func TestShouldRecordFix(t *testing.T) { last fixThrottle next chat1.Coordinate record bool - after fixThrottle }{ { name: "first fix since the watch started, in the background", state: keybase1.MobileAppState_BACKGROUND, next: origin, record: true, - after: fixThrottle{prev: at(origin)}, }, { name: "any move in the foreground", state: keybase1.MobileAppState_FOREGROUND, - last: fixThrottle{prev: at(origin), pendingDistance: 3}, + last: fixThrottle{lastRecorded: at(origin)}, next: north(origin, 1), record: true, - after: fixThrottle{prev: at(north(origin, 1))}, }, { name: "short move in the background", state: keybase1.MobileAppState_BACKGROUND, - last: fixThrottle{prev: at(origin)}, + last: fixThrottle{lastRecorded: at(origin)}, next: north(origin, 10), record: false, - after: fixThrottle{prev: at(north(origin, 10)), pendingDistance: 10}, }, { name: "short move while background work runs", state: keybase1.MobileAppState_BACKGROUNDACTIVE, - last: fixThrottle{prev: at(origin)}, + last: fixThrottle{lastRecorded: at(origin)}, next: north(origin, 10), record: false, - after: fixThrottle{prev: at(north(origin, 10)), pendingDistance: 10}, }, { name: "short move while on screen but not active", state: keybase1.MobileAppState_INACTIVE, - last: fixThrottle{prev: at(origin)}, + last: fixThrottle{lastRecorded: at(origin)}, next: north(origin, 10), record: false, - after: fixThrottle{prev: at(north(origin, 10)), pendingDistance: 10}, }, { name: "long move in the background", state: keybase1.MobileAppState_BACKGROUND, - last: fixThrottle{prev: at(origin)}, + last: fixThrottle{lastRecorded: at(origin)}, next: north(origin, 100), record: true, - after: fixThrottle{prev: at(north(origin, 100))}, }, { - name: "unrecorded moves add up to the distance", + name: "just past the distance", state: keybase1.MobileAppState_BACKGROUND, - last: fixThrottle{prev: at(origin), pendingDistance: 60}, - next: north(origin, 10), - record: true, - after: fixThrottle{prev: at(north(origin, 10))}, - }, - { - name: "the distance is along the path, not from the last recorded fix", - state: keybase1.MobileAppState_BACKGROUND, - last: fixThrottle{prev: at(north(origin, 40)), pendingDistance: 40}, - next: origin, - record: true, - after: fixThrottle{prev: at(origin)}, - }, - { - name: "exactly the distance", - state: keybase1.MobileAppState_BACKGROUND, - last: fixThrottle{prev: at(origin), pendingDistance: 65}, - next: origin, + last: fixThrottle{lastRecorded: at(origin)}, + next: north(origin, 65.1), record: true, - after: fixThrottle{prev: at(origin)}, }, { name: "just short of the distance", state: keybase1.MobileAppState_BACKGROUND, - last: fixThrottle{prev: at(origin), pendingDistance: 64.9}, - next: origin, + last: fixThrottle{lastRecorded: at(origin)}, + next: north(origin, 64.9), record: false, - after: fixThrottle{prev: at(origin), pendingDistance: 64.9}, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { record, after := shouldRecordFix(c.state, c.last, c.next) require.Equal(t, c.record, record) - require.Equal(t, c.after.prev, after.prev) - require.InDelta(t, c.after.pendingDistance, after.pendingDistance, 1e-6) + want := c.last + if c.record { + want.lastRecorded = at(c.next) + } + require.Equal(t, want, after) }) } } + +// recordedAt feeds fixes to a fresh throttle in the background and returns the +// indexes of the ones it records. +func recordedAt(fixes []chat1.Coordinate) (recorded []int) { + var throttle fixThrottle + for i, fix := range fixes { + var record bool + record, throttle = shouldRecordFix(keybase1.MobileAppState_BACKGROUND, throttle, fix) + if record { + recorded = append(recorded, i) + } + } + return recorded +} + +func TestShouldRecordFixIgnoresJitter(t *testing.T) { + origin := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194, Accuracy: 100} + fixes := []chat1.Coordinate{origin} + for i := 0; i < 20; i++ { + fixes = append(fixes, north(origin, 40), north(origin, -40)) + } + require.Equal(t, []int{0}, recordedAt(fixes)) +} + +func TestShouldRecordFixSlowDrift(t *testing.T) { + origin := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194, Accuracy: 10} + var fixes []chat1.Coordinate + for i := 0; i <= 14; i++ { + fixes = append(fixes, north(origin, float64(10*i))) + } + // 70m from origin at fix 7, then 70m from that at fix 14. + require.Equal(t, []int{0, 7, 14}, recordedAt(fixes)) +} From 068b46d10611b1d069b67e0bea016d0010fb4aa7 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:49:41 -0400 Subject: [PATCH 5/7] fix(chat): don't record a background fix that moved less than its accuracy The background throttle measured each fix against the last recorded one, but that anchor can itself be an outlier: a cold fix 40m off lets jitter on the other side clear 65m and become the new anchor, and a stationary phone keeps posting. Require a background fix to lie at least max(65m, anchor accuracy + next accuracy) from the anchor, so two fixes whose accuracy circles overlap never count as a move. Also record a fix less than half as uncertain as the anchor, so a coarse cold fix is replaced once the device locks on rather than holding the threshold wide open. An accuracy of 0 (unknown) never counts as an improvement. --- go/chat/maps/livelocation.go | 24 +++++++---- go/chat/maps/livelocation_throttle_test.go | 47 ++++++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/go/chat/maps/livelocation.go b/go/chat/maps/livelocation.go index a7ae7f04d665..98571647d28a 100644 --- a/go/chat/maps/livelocation.go +++ b/go/chat/maps/livelocation.go @@ -468,8 +468,8 @@ func (l *LiveLocationTracker) StartTracking(ctx context.Context, convID chat1.Co l.eg.Go(func() error { return l.tracker(t) }) } -// backgroundFixDistance is how far, in meters, the device must move before a -// native fix is recorded while the app is not in the foreground. +// backgroundFixDistance is the least distance, in meters, the device must move +// before a native fix is recorded while the app is not in the foreground. const backgroundFixDistance = 65 // earthRadiusMeters is the mean radius of the Earth. @@ -484,14 +484,24 @@ type fixThrottle struct { // shouldRecordFix decides whether a native fix gets recorded, and returns the // throttle to use for the next one. Out of the foreground a fix is recorded -// only once it lies backgroundFixDistance in a straight line from the last one -// recorded, so a stationary device's GPS jitter, which wanders back and forth -// around one spot, never adds up to a move. The first fix after the watch +// only once it lies, in a straight line from the last one recorded, at least +// backgroundFixDistance and at least both fixes' accuracies added together: +// closer than that, the two could be the same spot, so a stationary device's +// jitter never counts as a move, even when the fix it is measured from was +// itself an outlier. A fix less than half as uncertain as the last recorded +// one is recorded too, so a coarse cold fix gets replaced once the device +// locks on instead of holding the throttle wide open; an accuracy of 0 means +// unknown and never counts as better. The first fix after the watch // starts is recorded right away, so the move that relaunched the app gets // posted. func shouldRecordFix(state keybase1.MobileAppState, last fixThrottle, next chat1.Coordinate) (bool, fixThrottle) { - record := last.lastRecorded == nil || state == keybase1.MobileAppState_FOREGROUND || - distanceMeters(*last.lastRecorded, next) >= backgroundFixDistance + record := last.lastRecorded == nil || state == keybase1.MobileAppState_FOREGROUND + if !record { + anchor := *last.lastRecorded + minMove := math.Max(backgroundFixDistance, anchor.Accuracy+next.Accuracy) + record = distanceMeters(anchor, next) >= minMove || + (next.Accuracy > 0 && next.Accuracy < anchor.Accuracy/2) + } if record { last.lastRecorded = &next } diff --git a/go/chat/maps/livelocation_throttle_test.go b/go/chat/maps/livelocation_throttle_test.go index 0741ad69dc37..da59b03ddc27 100644 --- a/go/chat/maps/livelocation_throttle_test.go +++ b/go/chat/maps/livelocation_throttle_test.go @@ -16,6 +16,11 @@ func north(c chat1.Coordinate, meters float64) chat1.Coordinate { return c } +func withAccuracy(c chat1.Coordinate, accuracy float64) chat1.Coordinate { + c.Accuracy = accuracy + return c +} + func TestShouldRecordFix(t *testing.T) { origin := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194, Accuracy: 10} at := func(c chat1.Coordinate) *chat1.Coordinate { return &c } @@ -75,6 +80,27 @@ func TestShouldRecordFix(t *testing.T) { next: north(origin, 65.1), record: true, }, + { + name: "long move with a coarse fix", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(origin)}, + next: withAccuracy(north(origin, 100), 100), + record: false, + }, + { + name: "move past both accuracies", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(origin)}, + next: withAccuracy(north(origin, 111), 100), + record: true, + }, + { + name: "short move with unknown accuracy", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(origin)}, + next: withAccuracy(north(origin, 10), 0), + record: false, + }, { name: "just short of the distance", state: keybase1.MobileAppState_BACKGROUND, @@ -128,3 +154,24 @@ func TestShouldRecordFixSlowDrift(t *testing.T) { // 70m from origin at fix 7, then 70m from that at fix 14. require.Equal(t, []int{0, 7, 14}, recordedAt(fixes)) } + +func TestShouldRecordFixIgnoresJitterAroundOutlierAnchor(t *testing.T) { + center := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194} + fixes := []chat1.Coordinate{withAccuracy(north(center, 40), 100)} + for i := 0; i < 20; i++ { + fixes = append(fixes, + withAccuracy(north(center, -40), 65), + withAccuracy(north(center, 40), 100)) + } + require.Equal(t, []int{0}, recordedAt(fixes)) +} + +func TestShouldRecordFixReplacesCoarseAnchor(t *testing.T) { + center := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194, Accuracy: 10} + coarse := north(center, 300) + coarse.Accuracy = 1000 + fixes := []chat1.Coordinate{coarse, center, north(center, 20), north(center, -20), north(center, 70)} + // The locked-on fix replaces the coarse one, jitter around it is ignored, + // and a real move from it is recorded. + require.Equal(t, []int{0, 1, 4}, recordedAt(fixes)) +} From b70e25df0b3aa8d0dfb8c9b5b560c6cdc4a84de4 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 17:06:08 -0400 Subject: [PATCH 6/7] fix(chat): cap the background fix distance so coarse fixes still record A background fix had to move at least both fixes' accuracies added together. The iOS watcher asks for hundred-meter accuracy, and with Approximate Location every fix is kilometres wide, so the threshold grew to several kilometres and a drive across town recorded nothing. Cap the threshold at 200m. Realistic 65-100m accuracies stay under the cap, so jitter around an outlier anchor is still ignored. --- go/chat/maps/livelocation.go | 17 +++++++--- go/chat/maps/livelocation_throttle_test.go | 39 ++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/go/chat/maps/livelocation.go b/go/chat/maps/livelocation.go index 98571647d28a..6c715c04c1d9 100644 --- a/go/chat/maps/livelocation.go +++ b/go/chat/maps/livelocation.go @@ -472,6 +472,12 @@ func (l *LiveLocationTracker) StartTracking(ctx context.Context, convID chat1.Co // before a native fix is recorded while the app is not in the foreground. const backgroundFixDistance = 65 +// maxBackgroundFixDistance caps how far, in meters, the device must move before +// a background fix is recorded, however uncertain the fixes are. Approximate +// Location reports kilometres of uncertainty on every fix, so without a cap a +// drive across town would record nothing. +const maxBackgroundFixDistance = 200 + // earthRadiusMeters is the mean radius of the Earth. const earthRadiusMeters = 6371008.8 @@ -485,10 +491,10 @@ type fixThrottle struct { // shouldRecordFix decides whether a native fix gets recorded, and returns the // throttle to use for the next one. Out of the foreground a fix is recorded // only once it lies, in a straight line from the last one recorded, at least -// backgroundFixDistance and at least both fixes' accuracies added together: -// closer than that, the two could be the same spot, so a stationary device's -// jitter never counts as a move, even when the fix it is measured from was -// itself an outlier. A fix less than half as uncertain as the last recorded +// backgroundFixDistance and at least both fixes' accuracies added together, +// though never more than maxBackgroundFixDistance: closer than that, the two +// could be the same spot, so a stationary device's jitter never counts as a +// move, even when the fix it is measured from was itself an outlier. A fix less than half as uncertain as the last recorded // one is recorded too, so a coarse cold fix gets replaced once the device // locks on instead of holding the throttle wide open; an accuracy of 0 means // unknown and never counts as better. The first fix after the watch @@ -498,7 +504,8 @@ func shouldRecordFix(state keybase1.MobileAppState, last fixThrottle, next chat1 record := last.lastRecorded == nil || state == keybase1.MobileAppState_FOREGROUND if !record { anchor := *last.lastRecorded - minMove := math.Max(backgroundFixDistance, anchor.Accuracy+next.Accuracy) + minMove := math.Min(maxBackgroundFixDistance, + math.Max(backgroundFixDistance, anchor.Accuracy+next.Accuracy)) record = distanceMeters(anchor, next) >= minMove || (next.Accuracy > 0 && next.Accuracy < anchor.Accuracy/2) } diff --git a/go/chat/maps/livelocation_throttle_test.go b/go/chat/maps/livelocation_throttle_test.go index da59b03ddc27..bb2bdd01c806 100644 --- a/go/chat/maps/livelocation_throttle_test.go +++ b/go/chat/maps/livelocation_throttle_test.go @@ -94,6 +94,34 @@ func TestShouldRecordFix(t *testing.T) { next: withAccuracy(north(origin, 111), 100), record: true, }, + { + name: "coarse fixes, just short of the cap", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(withAccuracy(origin, 3000))}, + next: withAccuracy(north(origin, 199.9), 3000), + record: false, + }, + { + name: "coarse fixes, just past the cap", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(withAccuracy(origin, 3000))}, + next: withAccuracy(north(origin, 200.1), 3000), + record: true, + }, + { + name: "accuracies summing to just under the cap", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(withAccuracy(origin, 90))}, + next: withAccuracy(north(origin, 189), 100), + record: false, + }, + { + name: "accuracies summing to just under the cap, moved past them", + state: keybase1.MobileAppState_BACKGROUND, + last: fixThrottle{lastRecorded: at(withAccuracy(origin, 90))}, + next: withAccuracy(north(origin, 190.1), 100), + record: true, + }, { name: "short move with unknown accuracy", state: keybase1.MobileAppState_BACKGROUND, @@ -175,3 +203,14 @@ func TestShouldRecordFixReplacesCoarseAnchor(t *testing.T) { // and a real move from it is recorded. require.Equal(t, []int{0, 1, 4}, recordedAt(fixes)) } + +func TestShouldRecordFixCoarseFixesStillRecordMoves(t *testing.T) { + // Approximate Location reports every fix kilometres wide, so the fixes + // alone can never tell a move from jitter; a steady drive still records. + origin := chat1.Coordinate{Lat: 37.7749, Lon: -122.4194, Accuracy: 3000} + var fixes []chat1.Coordinate + for i := 0; i <= 4; i++ { + fixes = append(fixes, north(origin, float64(250*i))) + } + require.Equal(t, []int{0, 1, 2, 3, 4}, recordedAt(fixes)) +} From 290f59f0d880cf8e98755da6378272da56d44476 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 17:31:26 -0400 Subject: [PATCH 7/7] docs(chat): say what the capped fix distance gives up --- go/chat/maps/livelocation.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/go/chat/maps/livelocation.go b/go/chat/maps/livelocation.go index 6c715c04c1d9..6e2ed5f209b3 100644 --- a/go/chat/maps/livelocation.go +++ b/go/chat/maps/livelocation.go @@ -490,16 +490,17 @@ type fixThrottle struct { // shouldRecordFix decides whether a native fix gets recorded, and returns the // throttle to use for the next one. Out of the foreground a fix is recorded -// only once it lies, in a straight line from the last one recorded, at least -// backgroundFixDistance and at least both fixes' accuracies added together, -// though never more than maxBackgroundFixDistance: closer than that, the two -// could be the same spot, so a stationary device's jitter never counts as a -// move, even when the fix it is measured from was itself an outlier. A fix less than half as uncertain as the last recorded -// one is recorded too, so a coarse cold fix gets replaced once the device -// locks on instead of holding the throttle wide open; an accuracy of 0 means -// unknown and never counts as better. The first fix after the watch -// starts is recorded right away, so the move that relaunched the app gets -// posted. +// once it lies, in a straight line from the last one recorded, at least both +// fixes' accuracies added together: closer than that, the two could be the +// same spot, so jitter doesn't count as a move, even when the fix it is +// measured from was itself an outlier. That distance is kept between +// backgroundFixDistance and maxBackgroundFixDistance, so very coarse fixes +// (Approximate Location) still record a real move, at the cost of some of +// their jitter counting too. A fix less than half as uncertain as the last +// recorded one is recorded as well, so a coarse cold fix gets replaced once the +// device locks on; an accuracy of 0 means unknown and never counts as better. +// The first fix after the watch starts is recorded right away, so the move +// that relaunched the app gets posted. func shouldRecordFix(state keybase1.MobileAppState, last fixThrottle, next chat1.Coordinate) (bool, fixThrottle) { record := last.lastRecorded == nil || state == keybase1.MobileAppState_FOREGROUND if !record {