From 351109440ff2e796817c8ee385ea4617f66caa22 Mon Sep 17 00:00:00 2001 From: Dmitry Brant Date: Tue, 19 Mar 2019 15:59:33 -0400 Subject: [PATCH] Fix memory leak(s). (#2674) --- .../nrw/commons/category/GridViewAdapter.java | 8 ++------ .../contributions/ContributionsFragment.java | 8 ++------ .../contributions/ContributionsListAdapter.java | 12 ++++++------ .../commons/location/LocationServiceManager.java | 13 +++++++------ .../free/nrw/commons/nearby/NearbyFragment.java | 4 ++-- .../nearby/NearbyNotificationCardView.java | 16 +++++----------- 6 files changed, 24 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java b/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java index f9c7abb6e..196a5a686 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java @@ -1,6 +1,5 @@ package fr.free.nrw.commons.category; -import android.app.Activity; import android.content.Context; import android.text.TextUtils; import android.view.LayoutInflater; @@ -22,12 +21,10 @@ import fr.free.nrw.commons.R; */ public class GridViewAdapter extends ArrayAdapter { - private Context context; private List data; public GridViewAdapter(Context context, int layoutResourceId, List data) { super(context, layoutResourceId, data); - this.context = context; this.data = data; } @@ -81,8 +78,7 @@ public class GridViewAdapter extends ArrayAdapter { public View getView(int position, View convertView, ViewGroup parent) { if (convertView == null) { - LayoutInflater inflater = ((Activity) context).getLayoutInflater(); - convertView = inflater.inflate(R.layout.layout_category_images, null); + convertView = LayoutInflater.from(getContext()).inflate(R.layout.layout_category_images, null); } Media item = data.get(position); @@ -102,7 +98,7 @@ public class GridViewAdapter extends ArrayAdapter { */ private void setAuthorView(Media item, TextView author) { if (!TextUtils.isEmpty(item.getCreator())) { - String uploadedByTemplate = context.getString(R.string.image_uploaded_by); + String uploadedByTemplate = getContext().getString(R.string.image_uploaded_by); String uploadedBy = String.format(Locale.getDefault(), uploadedByTemplate, item.getCreator()); author.setText(uploadedBy); diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java index a5b69c17d..612a92e90 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java @@ -87,6 +87,7 @@ public class ContributionsFragment @Inject NearbyController nearbyController; @Inject OkHttpJsonApiClient okHttpJsonApiClient; @Inject CampaignsPresenter presenter; + @Inject LocationServiceManager locationManager; private ArrayList observersWaitingForLoad = new ArrayList<>(); private UploadService uploadService; @@ -105,8 +106,6 @@ public class ContributionsFragment private LatLng curLatLng; private boolean firstLocationUpdate = true; - public LocationServiceManager locationManager; - private boolean isFragmentAttachedBefore = false; private View checkBoxView; private CheckBox checkBox; @@ -496,7 +495,6 @@ public class ContributionsFragment @Override public void onResume() { super.onResume(); - locationManager = new LocationServiceManager(getActivity()); firstLocationUpdate = true; locationManager.addLocationListener(this); @@ -546,7 +544,7 @@ public class ContributionsFragment private void checkLocationPermission() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - if (locationManager.isLocationPermissionGranted()) { + if (locationManager.isLocationPermissionGranted(requireContext())) { nearbyNotificationCardView.permissionType = NearbyNotificationCardView.PermissionType.NO_PERMISSION_NEEDED; locationManager.registerLocationManager(); } else { @@ -638,8 +636,6 @@ public class ContributionsFragment getChildFragmentManager().removeOnBackStackChangedListener(this); locationManager.unregisterLocationManager(); locationManager.removeLocationListener(this); - // Try to prevent a possible NPE - locationManager.context = null; super.onDestroy(); if (isUploadServiceConnected) { diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java index 27674ed3c..db4b6cc50 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java @@ -2,6 +2,8 @@ package fr.free.nrw.commons.contributions; import android.content.Context; import android.database.Cursor; + +import androidx.annotation.NonNull; import androidx.cursoradapter.widget.CursorAdapter; import android.view.LayoutInflater; import android.view.View; @@ -20,14 +22,12 @@ class ContributionsListAdapter extends CursorAdapter { private final ContributionDao contributionDao; private UploadService uploadService; - private Context context; public ContributionsListAdapter(Context context, Cursor c, int flags, ContributionDao contributionDao) { super(context, c, flags); - this.context = context; this.contributionDao = contributionDao; } @@ -53,12 +53,12 @@ class ContributionsListAdapter extends CursorAdapter { new DisplayableContribution.ContributionActions() { @Override public void retryUpload() { - ContributionsListAdapter.this.retryUpload(contribution); + ContributionsListAdapter.this.retryUpload(view.getContext(), contribution); } @Override public void deleteUpload() { - ContributionsListAdapter.this.deleteUpload(contribution); + ContributionsListAdapter.this.deleteUpload(view.getContext(), contribution); } }); views.bindModel(context, displayableContribution); @@ -68,7 +68,7 @@ class ContributionsListAdapter extends CursorAdapter { * Retry upload when it is failed * @param contribution contribution to be retried */ - private void retryUpload(Contribution contribution) { + private void retryUpload(@NonNull Context context, Contribution contribution) { if (NetworkUtils.isInternetConnectionEstablished(context)) { if (contribution.getState() == STATE_FAILED && uploadService!= null) { @@ -87,7 +87,7 @@ class ContributionsListAdapter extends CursorAdapter { * Delete a failed upload attempt * @param contribution contribution to be deleted */ - private void deleteUpload(Contribution contribution) { + private void deleteUpload(@NonNull Context context, Contribution contribution) { if (NetworkUtils.isInternetConnectionEstablished(context)) { if (contribution.getState() == STATE_FAILED) { Timber.d("Deleting failed contrib %s", contribution.toString()); diff --git a/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java b/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java index f45cbd0c7..68b7c742d 100644 --- a/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java +++ b/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java @@ -9,6 +9,8 @@ import android.location.Location; import android.location.LocationListener; import android.location.LocationManager; import android.os.Bundle; + +import androidx.annotation.NonNull; import androidx.core.app.ActivityCompat; import androidx.core.content.ContextCompat; @@ -26,13 +28,12 @@ public class LocationServiceManager implements LocationListener { private static final long MIN_LOCATION_UPDATE_REQUEST_TIME_IN_MILLIS = 2 * 60 * 100; private static final long MIN_LOCATION_UPDATE_REQUEST_DISTANCE_IN_METERS = 10; - public Context context; private LocationManager locationManager; private Location lastLocation; //private Location lastLocationDuplicate; // Will be used for nearby card view on contributions activity private final List locationListeners = new CopyOnWriteArrayList<>(); private boolean isLocationManagerRegistered = false; - public Set locationExplanationDisplayed = new HashSet<>(); + private Set locationExplanationDisplayed = new HashSet<>(); /** * Constructs a new instance of LocationServiceManager. @@ -40,7 +41,6 @@ public class LocationServiceManager implements LocationListener { * @param context the context */ public LocationServiceManager(Context context) { - this.context = context; this.locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); } @@ -57,7 +57,7 @@ public class LocationServiceManager implements LocationListener { * Returns whether the location permission is granted. * @return true if the location permission is granted */ - public boolean isLocationPermissionGranted() { + public boolean isLocationPermissionGranted(@NonNull Context context) { return ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED; } @@ -101,8 +101,8 @@ public class LocationServiceManager implements LocationListener { * @return last known LatLng */ @SuppressLint("MissingPermission") - public LatLng getLKL() { - if (isLocationPermissionGranted()) { + public LatLng getLKL(@NonNull Context context) { + if (isLocationPermissionGranted(context)) { Location lastKL = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER); if (lastKL == null) { lastKL = locationManager.getLastKnownLocation(LocationManager.NETWORK_PROVIDER); @@ -225,6 +225,7 @@ public class LocationServiceManager implements LocationListener { */ public void unregisterLocationManager() { isLocationManagerRegistered = false; + locationExplanationDisplayed.clear(); try { locationManager.removeUpdates(this); } catch (SecurityException e) { diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java index c3b591768..3f7769fde 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java @@ -572,7 +572,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment */ private void registerLocationUpdates() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - if (locationManager.isLocationPermissionGranted()) { + if (locationManager.isLocationPermissionGranted(requireContext())) { locationManager.registerLocationManager(); } else { // Should we show an explanation? @@ -666,7 +666,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment private void checkLocationPermission() { Timber.d("Checking location permission"); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - if (locationManager.isLocationPermissionGranted()) { + if (locationManager.isLocationPermissionGranted(requireContext())) { refreshView(LOCATION_SIGNIFICANTLY_CHANGED); } else { // Should we show an explanation? diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNotificationCardView.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNotificationCardView.java index 651f18352..b5bc8b219 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNotificationCardView.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNotificationCardView.java @@ -21,9 +21,6 @@ import timber.log.Timber; * Custom card view for nearby notification card view on main screen, above contributions list */ public class NearbyNotificationCardView extends SwipableCardView { - - private Context context; - private Button permissionRequestButton; private LinearLayout contentLayout; private TextView notificationTitle; @@ -37,21 +34,18 @@ public class NearbyNotificationCardView extends SwipableCardView { public NearbyNotificationCardView(@NonNull Context context) { super(context); - this.context = context; cardViewVisibilityState = CardViewVisibilityState.INVISIBLE; init(); } public NearbyNotificationCardView(@NonNull Context context, @Nullable AttributeSet attrs) { super(context, attrs); - this.context = context; cardViewVisibilityState = CardViewVisibilityState.INVISIBLE; init(); } public NearbyNotificationCardView(@NonNull Context context, @Nullable AttributeSet attrs, int defStyleAttr) { super(context, attrs, defStyleAttr); - this.context = context; cardViewVisibilityState = CardViewVisibilityState.INVISIBLE; init(); } @@ -60,7 +54,7 @@ public class NearbyNotificationCardView extends SwipableCardView { * Initializes views and action listeners */ private void init() { - View rootView = inflate(context, R.layout.nearby_card_view, this); + View rootView = inflate(getContext(), R.layout.nearby_card_view, this); permissionRequestButton = rootView.findViewById(R.id.permission_request_button); contentLayout = rootView.findViewById(R.id.content_layout); @@ -79,7 +73,7 @@ public class NearbyNotificationCardView extends SwipableCardView { protected void onAttachedToWindow() { super.onAttachedToWindow(); // If you don't setVisibility after getting layout params, then you will se an empty space in place of nearby NotificationCardView - if (((MainActivity)context).defaultKvStore.getBoolean("displayNearbyCardView", true) && this.cardViewVisibilityState == NearbyNotificationCardView.CardViewVisibilityState.READY) { + if (((MainActivity)getContext()).defaultKvStore.getBoolean("displayNearbyCardView", true) && this.cardViewVisibilityState == NearbyNotificationCardView.CardViewVisibilityState.READY) { this.setVisibility(VISIBLE); } else { this.setVisibility(GONE); @@ -88,14 +82,14 @@ public class NearbyNotificationCardView extends SwipableCardView { private void setActionListeners() { - this.setOnClickListener(view -> ((MainActivity)context).viewPager.setCurrentItem(1)); + this.setOnClickListener(view -> ((MainActivity)getContext()).viewPager.setCurrentItem(1)); } @Override public boolean onSwipe(View view) { view.setVisibility(GONE); // Save shared preference for nearby card view accordingly - ((MainActivity) context).defaultKvStore.putBoolean("displayNearbyCardView", false); - ViewUtil.showLongToast(context, + ((MainActivity) getContext()).defaultKvStore.putBoolean("displayNearbyCardView", false); + ViewUtil.showLongToast(getContext(), getResources().getString(R.string.nearby_notification_dismiss_message)); return true; }