Merge pull request #942 from maskaravivek/leaks

Fix Leaks in Nearby activity

app/src/main/java/fr/free/nrw/commons/CommonsApplication.java

@@ -77,6 +77,8 @@ public class CommonsApplication extends Application implements HasActivityInject
     private DBOpenHelper dbOpenHelper = null;
     private NearbyPlaces nearbyPlaces = null;
 
+    private RefWatcher refWatcher;
+
     /**
      * This should not be called by ANY application code (other than the magic Android glue)
      * Use CommonsApplication.getInstance() instead to get the singleton.
@@ -128,7 +130,9 @@ public class CommonsApplication extends Application implements HasActivityInject
     public void onCreate() {
         super.onCreate();
 
-        setupLeakCanary();
+        if (setupLeakCanary() == RefWatcher.DISABLED) {
+            return;
+        }
 
         Timber.plant(new Timber.DebugTree());
 
@@ -160,6 +164,11 @@ public class CommonsApplication extends Application implements HasActivityInject
         return LeakCanary.install(this);
     }
 
+    public static RefWatcher getRefWatcher(Context context) {
+        CommonsApplication application = (CommonsApplication) context.getApplicationContext();
+        return application.refWatcher;
+    }
+
     /**
      * @return Account|null
      */

app/src/main/java/fr/free/nrw/commons/contributions/ContributionsActivity.java

@@ -38,6 +38,7 @@ import fr.free.nrw.commons.settings.Prefs;
 import fr.free.nrw.commons.upload.UploadService;
 import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.CompositeDisposable;
+import io.reactivex.disposables.Disposable;
 import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;
 
@@ -156,6 +157,7 @@ public class ContributionsActivity extends AuthenticatedActivity
         requestAuthToken();
         initDrawer();
         setTitle(getString(R.string.title_activity_contributions));
+        setUploadCount();
     }
 
     @Override
@@ -255,8 +257,6 @@ public class ContributionsActivity extends AuthenticatedActivity
             ((CursorAdapter) contributionsList.getAdapter()).swapCursor(cursor);
         }
 
-        setUploadCount();
-
         contributionsList.clearSyncMessage();
         notifyAndMigrateDataSetObservers();
     }
@@ -288,18 +288,17 @@ public class ContributionsActivity extends AuthenticatedActivity
     @SuppressWarnings("ConstantConditions")
     private void setUploadCount() {
         CommonsApplication app = ((CommonsApplication) getApplication());
-        compositeDisposable.add(
+        Disposable uploadCountDisposable = mediaWikiApi
-                mediaWikiApi
+                .getUploadCount(app.getCurrentAccount().name)
-                        .getUploadCount(app.getCurrentAccount().name)
+                .subscribeOn(Schedulers.io())
-                        .subscribeOn(Schedulers.io())
+                .observeOn(AndroidSchedulers.mainThread())
-                        .observeOn(AndroidSchedulers.mainThread())
+                .subscribe(
-                        .subscribe(
+                        uploadCount -> getSupportActionBar().setSubtitle(getResources()
-                                uploadCount -> getSupportActionBar().setSubtitle(getResources()
+                                .getQuantityString(R.plurals.contributions_subtitle,
-                                        .getQuantityString(R.plurals.contributions_subtitle,
+                                        uploadCount, uploadCount)),
-                                                uploadCount, uploadCount)),
+                        t -> Timber.e(t, "Fetching upload count failed")
-                                t -> Timber.e(t, "Fetching upload count failed")
+                );
-                        )
+        compositeDisposable.add(uploadCountDisposable);
-        );
     }
 
     @Override

app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java

@@ -7,6 +7,7 @@ import android.location.LocationListener;
 import android.location.LocationManager;
 import android.os.Bundle;
 
+import fr.free.nrw.commons.CommonsApplication;
 import timber.log.Timber;
 
 public class LocationServiceManager implements LocationListener {
@@ -16,11 +17,25 @@ public class LocationServiceManager implements LocationListener {
     private LatLng latestLocation;
     private Float latestLocationAccuracy;
 
-    public LocationServiceManager(Context context) {
+    private static LocationServiceManager locationServiceManager;
-        this.locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE);
+
+    private LocationServiceManager() {
+        Context applicationContext = CommonsApplication.getInstance().getApplicationContext();
+        this.locationManager = (LocationManager) applicationContext.getSystemService(Context.LOCATION_SERVICE);
         provider = locationManager.getBestProvider(new Criteria(), true);
     }
 
+    public static LocationServiceManager getInstance() {
+        if (locationServiceManager == null) {
+            locationServiceManager = new LocationServiceManager();
+        }
+        return locationServiceManager;
+    }
+
+    public boolean isProviderEnabled() {
+        return locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER);
+    }
+
     public LatLng getLatestLocation() {
         return latestLocation;
     }

app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java

@@ -5,9 +5,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
-import android.location.LocationManager;
 import android.net.Uri;
-import android.os.AsyncTask;
 import android.os.Build;
 import android.os.Bundle;
 import android.preference.PreferenceManager;
@@ -37,6 +35,10 @@ import fr.free.nrw.commons.location.LatLng;
 import fr.free.nrw.commons.location.LocationServiceManager;
 import fr.free.nrw.commons.theme.NavigationBaseActivity;
 import fr.free.nrw.commons.utils.UriSerializer;
+import io.reactivex.Observable;
+import io.reactivex.android.schedulers.AndroidSchedulers;
+import io.reactivex.disposables.Disposable;
+import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;
 
 
@@ -50,9 +52,9 @@ public class NearbyActivity extends NavigationBaseActivity {
     private LocationServiceManager locationManager;
     private LatLng curLatLang;
     private Bundle bundle;
-    private NearbyAsyncTask nearbyAsyncTask;
     private SharedPreferences sharedPreferences;
     private NearbyActivityMode viewMode;
+    private Disposable placesDisposable;
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {
@@ -60,6 +62,7 @@ public class NearbyActivity extends NavigationBaseActivity {
         sharedPreferences = PreferenceManager.getDefaultSharedPreferences(getApplicationContext());
         setContentView(R.layout.activity_nearby);
         ButterKnife.bind(this);
+        locationManager = LocationServiceManager.getInstance();
         checkLocationPermission();
         bundle = new Bundle();
         initDrawer();
@@ -105,11 +108,8 @@ public class NearbyActivity extends NavigationBaseActivity {
     }
 
     private void startLookingForNearby() {
-        locationManager = new LocationServiceManager(this);
-        locationManager.registerLocationManager();
         curLatLang = locationManager.getLatestLocation();
-        nearbyAsyncTask = new NearbyAsyncTask(this);
+        setupPlaceList(getBaseContext());
-        nearbyAsyncTask.execute();
     }
 
     private void checkLocationPermission() {
@@ -169,9 +169,6 @@ public class NearbyActivity extends NavigationBaseActivity {
                     startLookingForNearby();
                 } else {
                     //If permission not granted, go to page that says Nearby Places cannot be displayed
-                    if (nearbyAsyncTask != null) {
-                        nearbyAsyncTask.cancel(true);
-                    }
                     if (progressBar != null) {
                         progressBar.setVisibility(View.GONE);
                     }
@@ -200,8 +197,7 @@ public class NearbyActivity extends NavigationBaseActivity {
     }
 
     private void checkGps() {
-        LocationManager manager = (LocationManager) getSystemService(LOCATION_SERVICE);
+        if (!locationManager.isProviderEnabled()) {
-        if (!manager.isProviderEnabled(LocationManager.GPS_PROVIDER)) {
             Timber.d("GPS is not enabled");
             new AlertDialog.Builder(this)
                     .setMessage(R.string.gps_disabled)
@@ -231,16 +227,30 @@ public class NearbyActivity extends NavigationBaseActivity {
     }
 
     private void toggleView() {
-        if (nearbyAsyncTask != null) {
+        if (viewMode.isMap()) {
-            if (nearbyAsyncTask.getStatus() == AsyncTask.Status.FINISHED) {
+            setMapFragment();
-                if (viewMode.isMap()) {
+        } else {
-                    setMapFragment();
+            setListFragment();
-                } else {
-                    setListFragment();
-                }
-            }
-            sharedPreferences.edit().putBoolean(MAP_LAST_USED_PREFERENCE, viewMode.isMap()).apply();
         }
+        sharedPreferences.edit().putBoolean(MAP_LAST_USED_PREFERENCE, viewMode.isMap()).apply();
+    }
+
+    @Override
+    protected void onStart() {
+        super.onStart();
+        locationManager.registerLocationManager();
+    }
+
+    @Override
+    protected void onStop() {
+        super.onStop();
+        locationManager.unregisterLocationManager();
+    }
+
+    @Override
+    protected void onDestroy() {
+        super.onDestroy();
+        placesDisposable.dispose();
     }
 
     @Override
@@ -249,83 +259,48 @@ public class NearbyActivity extends NavigationBaseActivity {
         checkGps();
     }
 
-    @Override
-    protected void onPause() {
-        super.onPause();
-        if (nearbyAsyncTask != null) {
-            nearbyAsyncTask.cancel(true);
-        }
-    }
-
     private void refreshView() {
         curLatLang = locationManager.getLatestLocation();
         progressBar.setVisibility(View.VISIBLE);
-        nearbyAsyncTask = new NearbyAsyncTask(this);
+        setupPlaceList(getBaseContext());
-        nearbyAsyncTask.execute();
     }
 
-    @Override
+    private void setupPlaceList(Context context) {
-    protected void onDestroy() {
+        placesDisposable = Observable.fromCallable(() -> NearbyController
-        super.onDestroy();
+                .loadAttractionsFromLocation(curLatLang, CommonsApplication.getInstance()))
-        if (locationManager != null) {
+                .subscribeOn(Schedulers.io())
-            locationManager.unregisterLocationManager();
+                .observeOn(AndroidSchedulers.mainThread())
-        }
+                .subscribe((result) -> {
+                    populatePlaces(context, result);
+                });
     }
 
-    private class NearbyAsyncTask extends AsyncTask<Void, Integer, List<Place>> {
+    private void populatePlaces(Context context, List<Place> placeList) {
+        Gson gson = new GsonBuilder()
+                .registerTypeAdapter(Uri.class, new UriSerializer())
+                .create();
+        String gsonPlaceList = gson.toJson(placeList);
+        String gsonCurLatLng = gson.toJson(curLatLang);
 
-        private final Context mContext;
+        if (placeList.size() == 0) {
-
+            int duration = Toast.LENGTH_SHORT;
-        private NearbyAsyncTask(Context context) {
+            Toast toast = Toast.makeText(context, R.string.no_nearby, duration);
-            mContext = context;
+            toast.show();
         }
 
-        @Override
+        bundle.clear();
-        protected void onProgressUpdate(Integer... values) {
+        bundle.putString("PlaceList", gsonPlaceList);
-            super.onProgressUpdate(values);
+        bundle.putString("CurLatLng", gsonCurLatLng);
+
+        // Begin the transaction
+        if (viewMode.isMap()) {
+            setMapFragment();
+        } else {
+            setListFragment();
         }
 
-        @Override
+        if (progressBar != null) {
-        protected List<Place> doInBackground(Void... params) {
+            progressBar.setVisibility(View.GONE);
-            return NearbyController
-                    .loadAttractionsFromLocation(curLatLang, CommonsApplication.getInstance()
-                    );
-        }
-
-        @Override
-        protected void onPostExecute(List<Place> placeList) {
-            super.onPostExecute(placeList);
-
-            if (isCancelled()) {
-                return;
-            }
-
-            Gson gson = new GsonBuilder()
-                    .registerTypeAdapter(Uri.class, new UriSerializer())
-                    .create();
-            String gsonPlaceList = gson.toJson(placeList);
-            String gsonCurLatLng = gson.toJson(curLatLang);
-
-            if (placeList.size() == 0) {
-                int duration = Toast.LENGTH_SHORT;
-                Toast toast = Toast.makeText(mContext, R.string.no_nearby, duration);
-                toast.show();
-            }
-
-            bundle.clear();
-            bundle.putString("PlaceList", gsonPlaceList);
-            bundle.putString("CurLatLng", gsonCurLatLng);
-
-            // Begin the transaction
-            if (viewMode.isMap()) {
-                setMapFragment();
-            } else {
-                setListFragment();
-            }
-
-            if (progressBar != null) {
-                progressBar.setVisibility(View.GONE);
-            }
         }
     }
 

app/src/test/java/fr/free/nrw/commons/TestCommonsApplication.java

@@ -4,7 +4,8 @@ import com.squareup.leakcanary.RefWatcher;
 
 // This class is automatically discovered by Robolectric
 public class TestCommonsApplication extends CommonsApplication {
-    @Override protected RefWatcher setupLeakCanary() {
+    @Override
+    protected RefWatcher setupLeakCanary() {
         // No leakcanary in unit tests.
         return RefWatcher.DISABLED;
     }