Skip to content
This repository has been archived by the owner on Apr 23, 2023. It is now read-only.

Commit

Permalink
Simplify service shutdown to prevent crash on screen rotation (#180)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ecgreb @sarahsnow1
ecgreb authored and sarahsnow1 committed Apr 7, 2017
1 parent b59880d commit f9d8b9c
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 79 deletions.
4 changes: 2 additions & 2 deletions lost-sample/src/main/java/com/example/lost/MultipleLocationListenerSingleClientActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ private void initLocationTracking() {
return;
}

long interval = 3 * 60 * 1000; // 3 minutes
long interval = 30 * 1000; // 30 seconds
LocationRequest request = LocationRequest.create()
.setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY)
.setFastestInterval(interval)
.setInterval(interval);

LocationServices.FusedLocationApi.requestLocationUpdates(lostApiClient, request, listener);

interval = 30 * 1000; // 30 seconds
interval = 15 * 1000; // 15 seconds
request = LocationRequest.create()
.setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY)
.setFastestInterval(interval)
Expand Down
1 change: 0 additions & 1 deletion lost/src/main/java/com/mapzen/android/lost/internal/ClientManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ ReportedChanges sendPendingIntent(Context context, Location location,
void reportProviderDisabled(String provider);
void notifyLocationAvailability(final LocationAvailability availability);
boolean hasNoListeners();
void shutdown();
Map<LostApiClient, Set<LocationListener>> getLocationListeners();
Map<LostApiClient, Set<PendingIntent>> getPendingIntents();
Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks();
Expand Down
5 changes: 0 additions & 5 deletions lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ public FusedLocationProviderService getService() {
serviceImpl = new FusedLocationProviderServiceImpl(this, LostClientManager.shared());
}

@Override public void onDestroy() {
super.onDestroy();
serviceImpl.shutdown();
}

public Location getLastLocation(LostApiClient client) {
return serviceImpl.getLastLocation(client);
}
Expand Down
5 changes: 0 additions & 5 deletions lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ public FusedLocationProviderServiceImpl(Context context, ClientManager manager)
locationEngine = new FusionEngine(context, this);
}

public void shutdown() {
locationEngine.setRequest(null);
clientManager.shutdown();
}

public Location getLastLocation(LostApiClient client) {
return locationEngine.getLastLocation();
}
Expand Down
45 changes: 23 additions & 22 deletions lost/src/main/java/com/mapzen/android/lost/internal/LostClientManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import android.location.Location;
import android.os.Handler;
import android.os.Looper;
import android.support.annotation.VisibleForTesting;
import android.util.Log;

import java.util.HashMap;
Expand Down Expand Up @@ -55,37 +56,37 @@ public static LostClientManager shared() {
return instance;
}

public void addClient(LostApiClient client) {
@Override public void addClient(LostApiClient client) {
clients.put(client, new LostClientWrapper(client));
}

public void removeClient(LostApiClient client) {
@Override public void removeClient(LostApiClient client) {
clients.remove(client);
}

public boolean containsClient(LostApiClient client) {
@Override public boolean containsClient(LostApiClient client) {
return clients.containsKey(client);
}

public int numberOfClients() {
@Override public int numberOfClients() {
return clients.size();
}

public void addListener(LostApiClient client, LocationRequest request,
@Override public void addListener(LostApiClient client, LocationRequest request,
LocationListener listener) {
throwIfClientNotAdded(client);
clients.get(client).locationListeners().add(listener);
listenerToLocationRequests.put(listener, request);
}

public void addPendingIntent(LostApiClient client, LocationRequest request,
@Override public void addPendingIntent(LostApiClient client, LocationRequest request,
PendingIntent callbackIntent) {
throwIfClientNotAdded(client);
clients.get(client).pendingIntents().add(callbackIntent);
intentToLocationRequests.put(callbackIntent, request);
}

public void addLocationCallback(LostApiClient client, LocationRequest request,
@Override public void addLocationCallback(LostApiClient client, LocationRequest request,
LocationCallback callback, Looper looper) {
throwIfClientNotAdded(client);
clients.get(client).locationCallbacks().add(callback);
Expand All @@ -99,7 +100,7 @@ private void throwIfClientNotAdded(LostApiClient client) {
}
}

public boolean removeListener(LostApiClient client, LocationListener listener) {
@Override public boolean removeListener(LostApiClient client, LocationListener listener) {
final Set<LocationListener> listeners = clients.get(client).locationListeners();
boolean removed = false;

Expand All @@ -112,7 +113,7 @@ public boolean removeListener(LostApiClient client, LocationListener listener) {
return removed;
}

public boolean removePendingIntent(LostApiClient client, PendingIntent callbackIntent) {
@Override public boolean removePendingIntent(LostApiClient client, PendingIntent callbackIntent) {
final Set<PendingIntent> pendingIntents = clients.get(client).pendingIntents();
boolean removed = false;

Expand All @@ -125,7 +126,7 @@ public boolean removePendingIntent(LostApiClient client, PendingIntent callbackI
return removed;
}

public boolean removeLocationCallback(LostApiClient client, LocationCallback callback) {
@Override public boolean removeLocationCallback(LostApiClient client, LocationCallback callback) {
final Set<LocationCallback> callbacks = clients.get(client).locationCallbacks();
boolean removed = false;

Expand All @@ -148,7 +149,7 @@ public boolean removeLocationCallback(LostApiClient client, LocationCallback cal
* @param location
* @return
*/
public ReportedChanges reportLocationChanged(final Location location) {
@Override public ReportedChanges reportLocationChanged(final Location location) {
return iterateAndNotify(location, getLocationListeners(), listenerToLocationRequests,
new Notifier<LocationListener>() {
@Override public void notify(LostApiClient client, LocationListener listener) {
Expand All @@ -166,7 +167,7 @@ public ReportedChanges reportLocationChanged(final Location location) {
* @param location
* @return
*/
public ReportedChanges sendPendingIntent(final Context context,
@Override public ReportedChanges sendPendingIntent(final Context context,
final Location location, final LocationAvailability availability,
final LocationResult result) {
return iterateAndNotify(location,
Expand All @@ -177,7 +178,7 @@ public ReportedChanges sendPendingIntent(final Context context,
});
}

public ReportedChanges reportLocationResult(Location location,
@Override public ReportedChanges reportLocationResult(Location location,
final LocationResult result) {
return iterateAndNotify(location,
getLocationCallbacks(), callbackToLocationRequests, new Notifier<LocationCallback>() {
Expand All @@ -187,35 +188,35 @@ public ReportedChanges reportLocationResult(Location location,
});
}

public void updateReportedValues(ReportedChanges changes) {
@Override public void updateReportedValues(ReportedChanges changes) {
reportedChanges.putAll(changes);
}

public void reportProviderEnabled(String provider) {
@Override public void reportProviderEnabled(String provider) {
for (LostClientWrapper wrapper : clients.values()) {
for (LocationListener listener : wrapper.locationListeners()) {
listener.onProviderEnabled(provider);
}
}
}

public void reportProviderDisabled(String provider) {
@Override public void reportProviderDisabled(String provider) {
for (LostClientWrapper wrapper : clients.values()) {
for (LocationListener listener : wrapper.locationListeners()) {
listener.onProviderDisabled(provider);
}
}
}

public void notifyLocationAvailability(final LocationAvailability availability) {
@Override public void notifyLocationAvailability(final LocationAvailability availability) {
for (LostClientWrapper wrapper : clients.values()) {
for (LocationCallback callback : wrapper.locationCallbacks()) {
notifyAvailability(wrapper.client(), callback, availability);
}
}
}

public boolean hasNoListeners() {
@Override public boolean hasNoListeners() {
for (LostClientWrapper wrapper : clients.values()) {
if (!wrapper.locationListeners().isEmpty()
|| !wrapper.pendingIntents().isEmpty()
Expand All @@ -227,11 +228,11 @@ public boolean hasNoListeners() {
return true;
}

public void shutdown() {
@VisibleForTesting void clearClients() {
clients.clear();
}

public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
@Override public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
final Map<LostApiClient, Set<LocationListener>> clientToListeners = new HashMap<>();
for (LostApiClient client : clients.keySet()) {
clientToListeners.put(client, clients.get(client).locationListeners());
Expand All @@ -240,7 +241,7 @@ public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
return clientToListeners;
}

public Map<LostApiClient, Set<PendingIntent>> getPendingIntents() {
@Override public Map<LostApiClient, Set<PendingIntent>> getPendingIntents() {
final Map<LostApiClient, Set<PendingIntent>> clientToPendingIntents = new HashMap<>();
for (LostApiClient client : clients.keySet()) {
clientToPendingIntents.put(client, clients.get(client).pendingIntents());
Expand All @@ -249,7 +250,7 @@ public Map<LostApiClient, Set<PendingIntent>> getPendingIntents() {
return clientToPendingIntents;
}

public Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks() {
@Override public Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks() {
final Map<LostApiClient, Set<LocationCallback>> clientToLocationCallbacks = new HashMap<>();
for (LostApiClient client : clients.keySet()) {
clientToLocationCallbacks.put(client, clients.get(client).locationCallbacks());
Expand Down
23 changes: 0 additions & 23 deletions .../src/test/java/com/mapzen/android/lost/internal/FusedLocationProviderServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ public class FusedLocationProviderServiceImplTest extends BaseRobolectricTest {
@After public void tearDown() {
client.disconnect();
otherClient.disconnect();
clientManager.shutdown();
}

private void mockService() {
Expand Down Expand Up @@ -649,28 +648,6 @@ private File getTestGpxTrace() throws IOException {
assertThat(shadowLocationManager.getRequestLocationUpdateListeners()).isEmpty();
}

@Test public void shutdown_shouldUnregisterLocationUpdateListeners() throws Exception {
api.requestLocationUpdates(client, LocationRequest.create(),
new TestLocationListener());

api.shutdown();
assertThat(shadowLocationManager.getRequestLocationUpdateListeners()).isEmpty();
}

@Test public void shutdown_shouldClearListeners() {
api.requestLocationUpdates(client, LocationRequest.create(),
new TestLocationListener());
api.shutdown();
assertThat(api.getLocationListeners()).isEmpty();
}

@Test public void shutdown_shouldClearPendingIntents() {
api.requestLocationUpdates(client, LocationRequest.create(),
mock(PendingIntent.class));
api.shutdown();
assertThat(api.getPendingIntents()).isEmpty();
}

@Test public void requestLocationUpdates_shouldModifyOnlyClientListeners() {
client.connect();
api.requestLocationUpdates(client, LocationRequest.create(),
Expand Down
23 changes: 2 additions & 21 deletions lost/src/test/java/com/mapzen/android/lost/internal/LostClientManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@
@Config(constants = BuildConfig.class, sdk = 21, manifest = Config.NONE)
public class LostClientManagerTest extends BaseRobolectricTest {

ClientManager manager = LostClientManager.shared();
LostClientManager manager = LostClientManager.shared();
Context context = mock(Context.class);
LostApiClient client = new LostApiClient.Builder(context).build();

@After public void tearDown() {
manager.shutdown();
manager.clearClients();
}

@Test public void shouldHaveZeroClientCount() {
Expand Down Expand Up @@ -258,23 +258,4 @@ public void addLocationCallback_shouldThrowExceptionIfClientWasNotAdded() throws
manager.removeClient(client);
assertThat(manager.getLocationCallbacks().get(client)).isNull();
}

@Test public void shutdown_shouldClearAllMaps() {
manager.addClient(client);
LocationRequest request = LocationRequest.create();
TestLocationListener listener = new TestLocationListener();
manager.addListener(client, request, listener);

PendingIntent pendingIntent = mock(PendingIntent.class);
manager.addPendingIntent(client, request, pendingIntent);

TestLocationCallback callback = new TestLocationCallback();
Looper looper = mock(Looper.class);
manager.addLocationCallback(client, request, callback, looper);

manager.shutdown();
assertThat(manager.getLocationListeners()).isEmpty();
assertThat(manager.getPendingIntents()).isEmpty();
assertThat(manager.getLocationCallbacks()).isEmpty();
}
}

0 comments on commit f9d8b9c

Please sign in to comment.