Fix security exception (Nearby places not loading after permissions granted) (#1440)

* Update changelog.md

* Versioning for v2.7.0

* Add logging to onPermissionsRequestResult

* Request location updates onStatusChanged

* Copy onResume() actions into onPermissionsRequestResult

* Added getLastKnownLocation method and hooked it up to refreshView

* Remove unnecessary calls, add more logging

* Add check to prevent NPE

* Check that curLatLng exists before getting Mapbox instance

* Moar logging

* Make curLatLang clearer

* Not a good hack - put curLatLang into the bundle separately

* Add TODO

* Rename variables for clarity

* Check for Network Provider as well, tidy up getLKL()

* Add Javadocs

* Remove unnecessary method in onStatusChanged

* Add checkGPS comment

* Remove unnecessary logs

* Add TODO

* Call bundle.clear() before inserting CurLatLng

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

@@ -79,6 +79,23 @@ public class LocationServiceManager implements LocationListener {
                         Manifest.permission.ACCESS_FINE_LOCATION);
     }
 
+    /**
+     * Gets the last known location in cases where there wasn't time to register a listener
+     * (e.g. when Location permission just granted)
+     * @return last known LatLng
+     */
+    public LatLng getLKL() {
+        if (ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) {
+            Location lastKL = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER);
+            if (lastKL == null) {
+                lastKL = locationManager.getLastKnownLocation(LocationManager.NETWORK_PROVIDER);
+            }
+            return LatLng.from(lastKL);
+        } else {
+            return null;
+        }
+    }
+
     public LatLng getLastLocation() {
         if (lastLocation == null) {
             return null;
@@ -249,6 +266,7 @@ public class LocationServiceManager implements LocationListener {
     public enum LocationChangeType{
         LOCATION_SIGNIFICANTLY_CHANGED, //Went out of borders of nearby markers
         LOCATION_SLIGHTLY_CHANGED,      //User might be walking or driving
-        LOCATION_NOT_CHANGED
+        LOCATION_NOT_CHANGED,
+        PERMISSION_JUST_GRANTED
     }
 }

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

@@ -13,14 +13,12 @@ import android.support.design.widget.BottomSheetBehavior;
 
 import android.support.v4.app.FragmentTransaction;
 import android.support.v7.app.AlertDialog;
-import android.util.Log;
 import android.view.Menu;
 import android.view.MenuInflater;
 import android.view.MenuItem;
 import android.view.View;
 import android.widget.LinearLayout;
 import android.widget.ProgressBar;
-import android.widget.Toast;
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -81,6 +79,7 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp
 
     private final String NETWORK_INTENT_ACTION = "android.net.conn.CONNECTIVITY_CHANGE";
     private BroadcastReceiver broadcastReceiver;
+    private LatLng lastKnownLocation;
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {
@@ -158,7 +157,11 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp
         switch (requestCode) {
             case LOCATION_REQUEST: {
                 if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
-                    refreshView(LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED);
+                    Timber.d("Location permission granted, refreshing view");
+                    //Still need to check if GPS is enabled
+                    checkGps();
+                    lastKnownLocation = locationManager.getLKL();
+                    refreshView(LocationServiceManager.LocationChangeType.PERMISSION_JUST_GRANTED);
                 } else {
                     //If permission not granted, go to page that says Nearby Places cannot be displayed
                     hideProgressBar();
@@ -347,21 +350,33 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp
         }
         curLatLang = lastLocation;
 
+        if (locationChangeType.equals(LocationServiceManager.LocationChangeType.PERMISSION_JUST_GRANTED)) {
+            curLatLang = lastKnownLocation;
+        }
+
         if (curLatLang == null) {
             Timber.d("Skipping update of nearby places as location is unavailable");
             return;
         }
 
-        if (locationChangeType
-                .equals(LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED)) {
+        if (locationChangeType.equals(LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED)
+                || locationChangeType.equals(LocationServiceManager.LocationChangeType.PERMISSION_JUST_GRANTED)) {
             progressBar.setVisibility(View.VISIBLE);
+
+            //TODO: This hack inserts curLatLng before populatePlaces is called (see #1440). Ideally a proper fix should be found
+            Gson gson = new GsonBuilder()
+                    .registerTypeAdapter(Uri.class, new UriSerializer())
+                    .create();
+            String gsonCurLatLng = gson.toJson(curLatLang);
+            bundle.clear();
+            bundle.putString("CurLatLng", gsonCurLatLng);
+
             placesDisposable = Observable.fromCallable(() -> nearbyController
                     .loadAttractionsFromLocation(curLatLang))
                     .subscribeOn(Schedulers.io())
                     .observeOn(AndroidSchedulers.mainThread())
                     .subscribe(this::populatePlaces);
-        } else if (locationChangeType
-                .equals(LocationServiceManager.LocationChangeType.LOCATION_SLIGHTLY_CHANGED)) {
+        } else if (locationChangeType.equals(LocationServiceManager.LocationChangeType.LOCATION_SLIGHTLY_CHANGED)) {
             Gson gson = new GsonBuilder()
                     .registerTypeAdapter(Uri.class, new UriSerializer())
                     .create();
@@ -384,14 +399,14 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp
         if (placeList.size() == 0) {
             ViewUtil.showSnackbar(findViewById(R.id.container), R.string.no_nearby);
         }
-
-        bundle.clear();
+        
         bundle.putString("PlaceList", gsonPlaceList);
-        bundle.putString("CurLatLng", gsonCurLatLng);
+        //bundle.putString("CurLatLng", gsonCurLatLng);
         bundle.putString("BoundaryCoord", gsonBoundaryCoordinates);
 
         // First time to init fragments
         if (nearbyMapFragment == null) {
+            Timber.d("Init map fragment for the first time");
             lockNearbyView(true);
             setMapFragment();
             setListFragment();
@@ -399,6 +414,7 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp
             lockNearbyView(false);
         } else {
             // There are fragments, just update the map and list
+            Timber.d("Map fragment already exists, just update the map and list");
             updateMapFragment(false);
             updateListFragment();
         }

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

@@ -82,8 +82,11 @@ public class NearbyListFragment extends DaggerFragment {
     }
 
     public void updateNearbyListSignificantly() {
-        adapterFactory.updateAdapterData(getPlaceListFromBundle(bundleForUpdates),
-                (RVRendererAdapter<Place>) recyclerView.getAdapter());
+        try {
+            adapterFactory.updateAdapterData(getPlaceListFromBundle(bundleForUpdates), (RVRendererAdapter<Place>) recyclerView.getAdapter());
+        } catch (NullPointerException e) {
+            Timber.e("Null pointer exception from calling recyclerView.getAdapter()");
+        }
     }
 
     private List<Place> getPlaceListFromBundle(Bundle bundle) {

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

@@ -126,6 +126,7 @@ public class NearbyMapFragment extends DaggerFragment {
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
+        Timber.d("Nearby map fragment created");
 
         controller = new ContributionController(this);
         directUpload = new DirectUpload(this, controller);
@@ -151,9 +152,11 @@ public class NearbyMapFragment extends DaggerFragment {
                             getActivity());
             boundaryCoordinates = gson.fromJson(gsonBoundaryCoordinates, gsonBoundaryCoordinatesType);
         }
-        Mapbox.getInstance(getActivity(),
-                getString(R.string.mapbox_commons_app_token));
-        MapboxTelemetry.getInstance().setTelemetryEnabled(false);
+        if (curLatLng != null) {
+            Mapbox.getInstance(getActivity(),
+                    getString(R.string.mapbox_commons_app_token));
+            MapboxTelemetry.getInstance().setTelemetryEnabled(false);
+        }
         setRetainInstance(true);
     }
 
@@ -161,7 +164,9 @@ public class NearbyMapFragment extends DaggerFragment {
     public View onCreateView(LayoutInflater inflater, ViewGroup container,
                              Bundle savedInstanceState) {
 
+        Timber.d("onCreateView called");
         if (curLatLng != null) {
+            Timber.d("curLatLng found, setting up map view...");
             setupMapView(savedInstanceState);
         }