NPE in moveCamera

[RobinIsTheBird]

This issue relates to Rollbar #1880,

java.lang.NullPointerException: Attempt to invoke virtual method 'void com.google.android.gms.maps.GoogleMap.moveCamera(com.google.android.gms.maps.CameraUpdate)' on a null object reference
at org.azavea.otm.ui.MainMapFragment.lambda$null$3 (MainMapFragment.java:488)

The Rollbar traceback isn't entirely clear whether the null was the mMap or the return value of CameraUpdateFactory, but it looks to me like it's probably the former.

mMap would be null here as a result of a race condition between onCreateView, where the mapView that creates the mMap is setup, and the fragment onConnected, where the fragment attempts to zoom to the current location. Being a race condition would explain why I am unable to reproduce it.

[RobinIsTheBird]

In addition to the lifecycle fixes above, I intend to add a promise to join deferreds from the mapView.getMapAsync and onConnected together, and call moveToCurrentLocation only when the joint promise completes.

[RobinIsTheBird]

Here is a gliffy diagram showing dependencies between asynchronously performed procedures.

[RobinIsTheBird]

From a slack with @maurizi on 12/05/16:

mmaurizi [2:44 PM]
it seems like the 3 events we need to wrap into promises are: the callback of getMapAsync, the location services being ready, and the instance having loaded

rschaufler [2:45 PM]
yes.

[2:46]
Not everything requires all three, but the simplest code structure might be to just have one method that runs when all 3 are done, and not worry about excess synchronization.

mmaurizi [2:46 PM]
we can't do that - there are legitimate situations where location will never be ready

rschaufler [2:46 PM]
true.

mmaurizi [2:47 PM]
but we can combine map ready & instance loaded

rschaufler [2:47 PM]
we'd have to have a timeout that would reject location, and a variant of the DONE-DONE-DONE to deal w DONE-DONE-FAIL.

[2:48]
So maybe we want 3 methods.

mmaurizi [2:48 PM]
to get the best user experience, we likely want DONE-DONE (do reasonable default) - (DONE - move to GPS location / FAIL do nothing)

rschaufler [2:49 PM]
async-map && instance-loaded both DONE, async-map && instance-loaded DONE and location DONE, and async-map && instance-loaded DONE and location FAIL.

mmaurizi [2:49 PM]
yeah

[2:50]
I don't think this planned refactor is a 1-pointer though

rschaufler [2:50 PM]
agreed

mmaurizi [2:50 PM]
Probably should happen next sprint?

rschaufler [2:51 PM]
agreed. I feel I've already put in the 1-point of work to figure out what's going on.

another piece of refactor - moving it all into onActivityStarted (or whatever it's called)

mmaurizi [2:52 PM]
why would we move everything there?

[2:52]
I guess its as good a place as any?

rschaufler [2:53 PM]
getting the MapView requires the activity. It just happens to have worked in onCreateView, because it just happens to already have its activity, but the onActivityWhatever is more correct.

[2:53]
onActivityConnected ? (edited)

mmaurizi [2:54 PM]
onActivityCreate?onActivityCreated?

rschaufler [2:54 PM]
something like that. Whatever it is, it comes after onCreateView.