Fix Memory Leaks (#368)

* upgrade to lost 2.3.0 snapshot

* dont hold static reference to context in locationfactory

* use application context in mapzenmanager

* use weak reference in connection callbacks

* use mock context that has mock application context in tests

* cleanup listener in basic example

* checkstyle

Author: sarahcodes100

core/build.gradle

@@ -78,7 +78,7 @@ dependencies {
 
   compile "com.mapzen.tangram:tangram:$tangram_version"
   compile 'com.mapzen.android:pelias-android-sdk:1.1.0'
-  compile 'com.mapzen.android:lost:2.2.0'
+  compile 'com.mapzen.android:lost:2.3.0-SNAPSHOT'
 
   compile 'com.google.dagger:dagger:2.0'
   compile 'javax.annotation:javax.annotation-api:1.2'

core/src/main/java/com/mapzen/android/core/MapzenManager.java

@@ -36,7 +36,7 @@ public class MapzenManager {
    */
   public static MapzenManager instance(Context context) {
     if (instance == null) {
-      instance = new MapzenManager(context);
+      instance = new MapzenManager(context.getApplicationContext());
     }
 
     return instance;

core/src/main/java/com/mapzen/android/graphics/OverlayManager.java

@@ -19,6 +19,7 @@
 import android.view.View;
 import android.widget.ImageButton;
 
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -86,12 +87,6 @@ public class OverlayManager implements TouchInput.PanResponder, TouchInput.Rotat
     @Override public void onLocationChanged(Location location) {
       handleLocationChange(location);
     }
-
-    @Override public void onProviderEnabled(String provider) {
-    }
-
-    @Override public void onProviderDisabled(String provider) {
-    }
   };
 
   View.OnClickListener compassExternalClickListener;
@@ -116,19 +111,19 @@ public class OverlayManager implements TouchInput.PanResponder, TouchInput.Rotat
 
   private static class OverlayManagerConnectionCallbacks
       implements LostApiClient.ConnectionCallbacks {
-    private OverlayManager overlayManager;
+    private WeakReference<OverlayManager> overlayManager;
 
     @Override public void onConnected() {
-      if (overlayManager != null) {
-        overlayManager.enableLocationLayer();
+      if (overlayManager != null && overlayManager.get() != null) {
+        overlayManager.get().enableLocationLayer();
       }
     }
 
     @Override public void onConnectionSuspended() {
     }
 
     public void setOverlayManager(OverlayManager overlayManager) {
-      this.overlayManager = overlayManager;
+      this.overlayManager = new WeakReference(overlayManager);
     }
   };
 

core/src/main/java/com/mapzen/android/location/LocationFactory.java

@@ -11,7 +11,6 @@
 public class LocationFactory {
 
   private static LostApiClient shared;
-  private static Context context;
 
   /**
    * Returns shared {@link LostApiClient}.
@@ -23,9 +22,8 @@ public class LocationFactory {
    * and future attempts to bind the fused location service would fail.
    */
   @Deprecated public static LostApiClient sharedClient(Context context) {
-    if (LocationFactory.context != context) {
+    if (shared == null) {
       shared = new LostApiClient.Builder(context).build();
-      LocationFactory.context = context;
     }
 
     return shared;
@@ -42,9 +40,8 @@ public class LocationFactory {
    */
   @Deprecated public static LostApiClient sharedClient(Context context,
       ConnectionCallbacks callbacks) {
-    if (LocationFactory.context != context) {
+    if (shared == null) {
       shared = new LostApiClient.Builder(context).addConnectionCallbacks(callbacks).build();
-      LocationFactory.context = context;
     }
 
     return shared;

core/src/test/java/com/mapzen/android/core/MapzenManagerTest.java

@@ -13,10 +13,10 @@
 import android.content.res.Resources;
 import android.support.annotation.NonNull;
 
+import static com.mapzen.TestHelper.getMockContext;
 import static com.mapzen.android.core.MapzenManager.API_KEY_RES_NAME;
 import static com.mapzen.android.core.MapzenManager.API_KEY_RES_TYPE;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @RunWith(PowerMockRunner.class)
@@ -27,7 +27,7 @@ public class MapzenManagerTest {
   }
 
   @Test public void shouldNotBeNull() throws Exception {
-    Context context = mock(Context.class);
+    Context context = getMockContext();
     Resources resources = new TestResources();
     when(context.getResources()).thenReturn(resources);
     MapzenManager mapzenManager = MapzenManager.instance(context);
@@ -36,7 +36,7 @@ public class MapzenManagerTest {
 
   @Test(expected = IllegalStateException.class)
   public void getApiKey_shouldReturnThrowIfNotSet() throws Exception {
-    Context context = mock(Context.class);
+    Context context = getMockContext();
     Resources resources = new TestResources();
     when(context.getResources()).thenReturn(resources);
     MapzenManager mapzenManager = MapzenManager.instance(context);
@@ -45,15 +45,15 @@ public void getApiKey_shouldReturnThrowIfNotSet() throws Exception {
 
   @Test(expected = IllegalStateException.class)
   public void getApiKey_shouldReturnThrowIfResourceNotFound() throws Exception {
-    Context context = mock(Context.class);
+    Context context = getMockContext();
     Resources resources = new TestResourcesNotFound();
     when(context.getResources()).thenReturn(resources);
     MapzenManager mapzenManager = MapzenManager.instance(context);
     mapzenManager.getApiKey();
   }
 
   @Test public void getApiKey_shouldReturnStringResourceValue() throws Exception {
-    Context context = mock(Context.class);
+    Context context = getMockContext();
     TestResources resources = new TestResources();
     resources.testApiKey = "mapzen-fake-api-key";
     when(context.getResources()).thenReturn(resources);
@@ -62,7 +62,7 @@ public void getApiKey_shouldReturnThrowIfResourceNotFound() throws Exception {
   }
 
   @Test public void setApiKey_shouldOverrideStringResourcesValue() throws Exception {
-    Context context = mock(Context.class);
+    Context context = getMockContext();
     TestResources resources = new TestResources();
     resources.testApiKey = "mapzen-fake-api-key";
     when(context.getResources()).thenReturn(resources);

core/src/test/java/com/mapzen/android/graphics/MapInitializerTest.java

@@ -49,7 +49,11 @@ public class MapInitializerTest {
 
   @Test public void init_shouldReturnMapzenMap() throws Exception {
     final TestCallback callback = new TestCallback();
-    final TestMapView mapView = new TestMapView();
+    final TestMapView mapView = mock(TestMapView.class);
+    Context context = mock(Context.class);
+    when(context.getApplicationContext()).thenReturn(mock(Context.class));
+    when(mapView.getContext()).thenReturn(context);
+    when(mapView.getTangramMapView()).thenCallRealMethod();
     MapzenManager.instance(getMockContext()).setApiKey("fake-mapzen-api-key");
     mapInitializer.init(mapView, callback);
     assertThat(callback.map).isInstanceOf(MapzenMap.class);

core/src/test/java/com/mapzen/android/location/LocationFactoryTest.java

@@ -38,16 +38,16 @@ public class LocationFactoryTest {
         .isEqualTo(LocationFactory.sharedClient(context, callbacks));
   }
 
-  @Test public void sharedClient_shouldReturnNewClientForNewContext() throws Exception {
+  @Test public void sharedClient_shouldReturnSameClientForNewContext() throws Exception {
     assertThat(LocationFactory.sharedClient(Mockito.mock(Context.class)))
-        .isNotEqualTo(LocationFactory.sharedClient(Mockito.mock(Context.class)));
+        .isEqualTo(LocationFactory.sharedClient(Mockito.mock(Context.class)));
   }
 
-  @Test public void sharedClientWithCallbacks_shouldReturnNewClientForNewContext()
+  @Test public void sharedClientWithCallbacks_shouldReturnSameClientForNewContext()
       throws Exception {
     LostApiClient.ConnectionCallbacks callbacks = new TestConnectionCallbacks();
     assertThat(LocationFactory.sharedClient(Mockito.mock(Context.class), callbacks))
-        .isNotEqualTo(LocationFactory.sharedClient(Mockito.mock(Context.class), callbacks));
+        .isEqualTo(LocationFactory.sharedClient(Mockito.mock(Context.class), callbacks));
   }
 
   private class TestConnectionCallbacks implements LostApiClient.ConnectionCallbacks {

mapzen-places-api/src/test/java/com/mapzen/android/lost/internal/FusedApiServiceUpdater.java

@@ -0,0 +1,13 @@
+package com.mapzen.android.lost.internal;
+
+/**
+ * Created by sarahlensing on 5/11/17.
+ */
+
+public class FusedApiServiceUpdater {
+
+  public static void updateApiService(FusedLocationProviderApiImpl api,
+      IFusedLocationProviderService service) {
+    api.service = service;
+  }
+}

mapzen-places-api/src/test/java/com/mapzen/places/api/internal/PlaceAutocompletePresenterTest.java

@@ -2,8 +2,9 @@
 
 import com.mapzen.android.lost.api.LocationServices;
 import com.mapzen.android.lost.api.LostApiClient;
+import com.mapzen.android.lost.internal.FusedApiServiceUpdater;
 import com.mapzen.android.lost.internal.FusedLocationProviderApiImpl;
-import com.mapzen.android.lost.internal.FusedLocationProviderService;
+import com.mapzen.android.lost.internal.IFusedLocationProviderService;
 import com.mapzen.pelias.BoundingBox;
 import com.mapzen.pelias.gson.Feature;
 import com.mapzen.pelias.gson.Properties;
@@ -56,21 +57,20 @@ public void getBoundingBox_shouldReturnCorrectBox() throws Exception {
 
   @Test
   public void getBoundingBox_lostClient_shouldReturnCorrectBox() throws Exception {
-    FusedLocationProviderService.FusedLocationProviderBinder stubBinder =
-          mock(FusedLocationProviderService.FusedLocationProviderBinder.class);
-    FusedLocationProviderService mockService = mock(FusedLocationProviderService.class);
-      when(stubBinder.getService()).thenReturn(mockService);
     FusedLocationProviderApiImpl impl =
         (FusedLocationProviderApiImpl) LocationServices.FusedLocationApi;
-    impl.onServiceConnected(stubBinder);
+    IFusedLocationProviderService service = mock(IFusedLocationProviderService.class);
+    FusedApiServiceUpdater.updateApiService(impl, service);
 
     LostApiClient client = mock(LostApiClient.class);
+    when(client.isConnected()).thenReturn(true);
+    presenter.setLostClient(client);
+
     Location location = mock(Location.class);
     when(location.getLatitude()).thenReturn(40.0);
     when(location.getLongitude()).thenReturn(70.0);
-    when(mockService.getLastLocation(client)).thenReturn(location);
-    presenter.setLostClient(client);
-    when(client.isConnected()).thenReturn(true);
+    when(service.getLastLocation()).thenReturn(location);
+
     BoundingBox boundingBox = presenter.getBoundingBox();
     double boundsRadius = 0.02;
     assertThat(boundingBox.getMinLat() + boundsRadius).isEqualTo(location.getLatitude());

samples/mapzen-android-sdk-sample/src/main/java/com/mapzen/android/sdk/sample/BasicMapzenActivity.java

@@ -87,6 +87,11 @@ private void configureMap(boolean enabled) {
     }
   }
 
+  @Override protected void onDestroy() {
+    super.onDestroy();
+    map.setFindMeOnClickListener(null);
+  }
+
   @Override public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
     if (map == null) {
       return;