Skip to content

Commit 5f002f5

Browse files
committed
Prevent memory leak(s) in LocationServiceManager.
This was causing a memory leak in at least API 19, and possibly others.
1 parent f4807a5 commit 5f002f5

File tree

3 files changed

+11
-14
lines changed

3 files changed

+11
-14
lines changed

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public class ContributionsFragment
8787
@Inject NearbyController nearbyController;
8888
@Inject OkHttpJsonApiClient okHttpJsonApiClient;
8989
@Inject CampaignsPresenter presenter;
90+
@Inject LocationServiceManager locationManager;
9091

9192
private ArrayList<DataSetObserver> observersWaitingForLoad = new ArrayList<>();
9293
private UploadService uploadService;
@@ -105,8 +106,6 @@ public class ContributionsFragment
105106
private LatLng curLatLng;
106107

107108
private boolean firstLocationUpdate = true;
108-
public LocationServiceManager locationManager;
109-
110109
private boolean isFragmentAttachedBefore = false;
111110
private View checkBoxView;
112111
private CheckBox checkBox;
@@ -496,7 +495,6 @@ public void onSaveInstanceState(Bundle outState) {
496495
@Override
497496
public void onResume() {
498497
super.onResume();
499-
locationManager = new LocationServiceManager(getActivity());
500498

501499
firstLocationUpdate = true;
502500
locationManager.addLocationListener(this);
@@ -546,7 +544,7 @@ private void checkGPS() {
546544

547545
private void checkLocationPermission() {
548546
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
549-
if (locationManager.isLocationPermissionGranted()) {
547+
if (locationManager.isLocationPermissionGranted(requireContext())) {
550548
nearbyNotificationCardView.permissionType = NearbyNotificationCardView.PermissionType.NO_PERMISSION_NEEDED;
551549
locationManager.registerLocationManager();
552550
} else {
@@ -638,8 +636,6 @@ public void onDestroy() {
638636
getChildFragmentManager().removeOnBackStackChangedListener(this);
639637
locationManager.unregisterLocationManager();
640638
locationManager.removeLocationListener(this);
641-
// Try to prevent a possible NPE
642-
locationManager.context = null;
643639
super.onDestroy();
644640

645641
if (isUploadServiceConnected) {

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import android.location.LocationListener;
1010
import android.location.LocationManager;
1111
import android.os.Bundle;
12+
13+
import androidx.annotation.NonNull;
1214
import androidx.core.app.ActivityCompat;
1315
import androidx.core.content.ContextCompat;
1416

@@ -26,21 +28,19 @@ public class LocationServiceManager implements LocationListener {
2628
private static final long MIN_LOCATION_UPDATE_REQUEST_TIME_IN_MILLIS = 2 * 60 * 100;
2729
private static final long MIN_LOCATION_UPDATE_REQUEST_DISTANCE_IN_METERS = 10;
2830

29-
public Context context;
3031
private LocationManager locationManager;
3132
private Location lastLocation;
3233
//private Location lastLocationDuplicate; // Will be used for nearby card view on contributions activity
3334
private final List<LocationUpdateListener> locationListeners = new CopyOnWriteArrayList<>();
3435
private boolean isLocationManagerRegistered = false;
35-
public Set<Activity> locationExplanationDisplayed = new HashSet<>();
36+
private Set<Activity> locationExplanationDisplayed = new HashSet<>();
3637

3738
/**
3839
* Constructs a new instance of LocationServiceManager.
3940
*
4041
* @param context the context
4142
*/
4243
public LocationServiceManager(Context context) {
43-
this.context = context;
4444
this.locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE);
4545
}
4646

@@ -57,7 +57,7 @@ public boolean isProviderEnabled() {
5757
* Returns whether the location permission is granted.
5858
* @return true if the location permission is granted
5959
*/
60-
public boolean isLocationPermissionGranted() {
60+
public boolean isLocationPermissionGranted(@NonNull Context context) {
6161
return ContextCompat.checkSelfPermission(context,
6262
Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED;
6363
}
@@ -101,8 +101,8 @@ public boolean isPermissionExplanationRequired(Activity activity) {
101101
* @return last known LatLng
102102
*/
103103
@SuppressLint("MissingPermission")
104-
public LatLng getLKL() {
105-
if (isLocationPermissionGranted()) {
104+
public LatLng getLKL(@NonNull Context context) {
105+
if (isLocationPermissionGranted(context)) {
106106
Location lastKL = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER);
107107
if (lastKL == null) {
108108
lastKL = locationManager.getLastKnownLocation(LocationManager.NETWORK_PROVIDER);
@@ -225,6 +225,7 @@ private boolean isSameProvider(String provider1, String provider2) {
225225
*/
226226
public void unregisterLocationManager() {
227227
isLocationManagerRegistered = false;
228+
locationExplanationDisplayed.clear();
228229
try {
229230
locationManager.removeUpdates(this);
230231
} catch (SecurityException e) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ private void hideProgressBar() {
572572
*/
573573
private void registerLocationUpdates() {
574574
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
575-
if (locationManager.isLocationPermissionGranted()) {
575+
if (locationManager.isLocationPermissionGranted(requireContext())) {
576576
locationManager.registerLocationManager();
577577
} else {
578578
// Should we show an explanation?
@@ -666,7 +666,7 @@ private void checkGps() {
666666
private void checkLocationPermission() {
667667
Timber.d("Checking location permission");
668668
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
669-
if (locationManager.isLocationPermissionGranted()) {
669+
if (locationManager.isLocationPermissionGranted(requireContext())) {
670670
refreshView(LOCATION_SIGNIFICANTLY_CHANGED);
671671
} else {
672672
// Should we show an explanation?

0 commit comments

Comments
 (0)