Mapbox Android, MapView, possible unnecessary race condition in NativeMapView creation.

[jonl-percsolutions-com]

Platform: Android
Mapbox SDK version: tested 3.2.0 - 4.2.0-beta.5

Steps to trigger behavior

1. Add the MapView directly to a layout
2. Delay calling MapView.onCreate() long enough so that SurfaceCallback is triggered before nativeMapView.initializeDisplay()/nativeMapView.initializeContext().

Expected behavior

MapView displays and appears correctly.

Actual behavior

Call to nativeMapView.createSurface() call in SurfaceCallback.surfaceCreated(SurfaceHolder) causes a catastrophic failure, crashing the application with an error 3008 EGL_BAD_DISPLAY.

Details:

My application does some initial processing on load and while the view is created by adding it to the Layout it is not visible and initially onCreate was being called later than the surfaceCreated callback method. However, the chain of events causing the failure appears to be unnecessary and could be mitigated to make sure that the proper sequence of events has occurred to initialize the nativeMapView instead of allowing the application to crash with little to no feedback. This may be related to some issues people have reported in issue #7044.

Current sequence of events is:

1. Create MapView
2. In MapView.initialize(Context,MapboxMapOptions), load SurfaceView and add SurfaceCallback()
3. In MapView.initialize(Context,MapboxMapOptions), create NativeMapView();
4. Call MapView.onCreate(Bundle)
5. In MapView.onCreate() call nativeMapView.initializeDisplay();
6. In MapView.onCreate() call nativeMapView.initializeContext();
7. In SurveView.surfaceCreated call nativeMapView.creatSurves(Surface);

This sequence of events is not 100% guaranteed to occur in a specific order. Two possible solutions would be:

1. add a set of checks and state tracking attributes to verify that the nativeMapView is in the correct state before calling the various nativeMapView methods,
2. move the addition of the SurveCallback to the SurfaceView to the onCreate method after the call to nativeMapView.initializeContext();

Making these changes would make the loading of the MapView more robust and less likely to crash the application. Additional feedback could likely be added to the MapView to inform a developer if something in the sequence has not occurred correctly, allowing the developer to more easily fix the problem, instead of searching the internet for possible solutions.

[tobrun]

Thank you for taking the time to write this up so detailed! A side-note without looking into the issue is that you are required to hook into onCreate from Activity#onCreate. This is not optional or neither are you free to call this whenever you want. If this doesn't fit your app flow, you need to adjust you app flow to handle this correctly (eg. use an intermediate activity to do the preprocessing). That said, will 👀 this in more detail to make the SDK initialization as robust as possible.

[tobrun]

Great analysis, I'm trying out the latter option since the former would introduce additional state.

[jonl-percsolutions-com]

Thanks for looking into this and the quick. I have reworked my flow to make sure the map life cycle is called at a the appropriate times.

[tobrun]

Closed with #8759