[android] - don't invoke nativeRender when surface has been destroyed

7 files changed, 73 insertions, 19 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
index 238c79392b..82412c4386 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
@@ -10,6 +10,7 @@ import android.support.annotation.CallSuper;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.UiThread;
+import android.support.annotation.VisibleForTesting;
 import android.support.v4.util.LongSparseArray;
 import android.util.AttributeSet;
 import android.view.KeyEvent;
@@ -1067,6 +1068,12 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback {
     this.mapboxMap = mapboxMap;
   }
 
+  @VisibleForTesting
+  @Nullable
+  MapRenderer getMapRenderer() {
+    return mapRenderer;
+  }
+
   private class FocalPointInvalidator implements FocalPointChangeListener {
 
     private final List<FocalPointChangeListener> focalPointChangeListeners = new ArrayList<>();
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java
index 715d05df6f..0df72ab70b 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java
@@ -4,13 +4,12 @@ import android.content.Context;
 import android.support.annotation.CallSuper;
 import android.support.annotation.Keep;
 import android.support.annotation.NonNull;
+import android.support.annotation.UiThread;
 import com.mapbox.mapboxsdk.LibraryLoader;
 import com.mapbox.mapboxsdk.log.Logger;
 import com.mapbox.mapboxsdk.maps.MapboxMap;
 import com.mapbox.mapboxsdk.storage.FileSource;
 
-import java.util.concurrent.atomic.AtomicBoolean;
-
 import javax.microedition.khronos.egl.EGLConfig;
 import javax.microedition.khronos.opengles.GL10;
 
@@ -34,7 +33,8 @@ public abstract class MapRenderer implements MapRendererScheduler {
   private long nativePtr = 0;
   private double expectedRenderTime = 0;
   private MapboxMap.OnFpsChangedListener onFpsChangedListener;
-  protected AtomicBoolean hasSurface = new AtomicBoolean();
+  protected boolean hasSurface;
+  protected final Object lock = new Object();
 
   public MapRenderer(@NonNull Context context, String localIdeographFontFamily) {
     float pixelRatio = context.getResources().getDisplayMetrics().density;
@@ -88,7 +88,14 @@ public abstract class MapRenderer implements MapRendererScheduler {
   protected void onDrawFrame(GL10 gl) {
     long startTime = System.nanoTime();
     try {
-      nativeRender();
+      synchronized (lock) {
+        if (hasSurface) {
+          nativeRender();
+        } else {
+          return;
+        }
+      }
+
     } catch (java.lang.Error error) {
       Logger.e(TAG, error.getMessage());
     }
@@ -166,7 +173,8 @@ public abstract class MapRenderer implements MapRendererScheduler {
    *
    * @return returns if renderer has a surface, false otherwise
    */
+  @UiThread
   public boolean hasSurface() {
-    return hasSurface.get();
+    return hasSurface;
   }
 }
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java
index 9ddee8d1ad..0f266a2cfb 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java
@@ -4,7 +4,6 @@ import android.content.Context;
 import android.opengl.GLSurfaceView;
 import android.support.annotation.NonNull;
 import android.view.SurfaceHolder;
-
 import com.mapbox.mapboxsdk.maps.renderer.MapRenderer;
 import com.mapbox.mapboxsdk.maps.renderer.egl.EGLConfigChooser;
 
@@ -39,14 +38,18 @@ public class GLSurfaceViewMapRenderer extends MapRenderer implements GLSurfaceVi
       @Override
       public void surfaceCreated(SurfaceHolder holder) {
         super.surfaceCreated(holder);
-        hasSurface.set(true);
+        synchronized (lock) {
+          hasSurface = true;
+        }
       }
 
       @Override
       public void surfaceDestroyed(SurfaceHolder holder) {
         super.surfaceDestroyed(holder);
-        hasSurface.set(false);
-        nativeReset();
+        synchronized (lock) {
+          hasSurface = false;
+          nativeReset();
+        }
       }
     });
   }
@@ -103,10 +106,11 @@ public class GLSurfaceViewMapRenderer extends MapRenderer implements GLSurfaceVi
    */
   @Override
   public void requestRender() {
-    if (!hasSurface.get()) {
-      return;
+    synchronized (lock) {
+      if (hasSurface) {
+        glSurfaceView.requestRender();
+      }
     }
-    glSurfaceView.requestRender();
   }
 
   /**
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java
index b60316e586..88c29d6cbf 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java
@@ -42,8 +42,10 @@ public class TextureViewMapRenderer extends MapRenderer {
    */
   @Override
   protected void onSurfaceCreated(GL10 gl, EGLConfig config) {
-    super.onSurfaceCreated(gl, config);
-    hasSurface.set(true);
+    synchronized (lock) {
+      super.onSurfaceCreated(gl, config);
+      hasSurface = true;
+    }
   }
 
   /**
@@ -59,8 +61,10 @@ public class TextureViewMapRenderer extends MapRenderer {
    */
   @Override
   protected void onSurfaceDestroyed() {
-    hasSurface.set(false);
-    super.onSurfaceDestroyed();
+    synchronized (lock) {
+      hasSurface = false;
+      super.onSurfaceDestroyed();
+    }
   }
 
   /**
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/build.gradle b/platform/android/MapboxGLAndroidSDKTestApp/build.gradle
index ca280436ea..82244bbe50 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/build.gradle
+++ b/platform/android/MapboxGLAndroidSDKTestApp/build.gradle
@@ -42,6 +42,7 @@ android {
             proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'
         }
         release {
+            debuggable true
             minifyEnabled true
             shrinkResources true
             proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/GLSurfaceDestroyTest.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/GLSurfaceDestroyTest.kt
new file mode 100644
index 0000000000..0ca00809d6
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/maps/GLSurfaceDestroyTest.kt
@@ -0,0 +1,32 @@
+package com.mapbox.mapboxsdk.maps
+
+import android.support.test.filters.LargeTest
+import android.support.test.rule.ActivityTestRule
+import android.support.test.runner.AndroidJUnit4
+import com.mapbox.mapboxsdk.integration.BaseIntegrationTest
+import com.mapbox.mapboxsdk.maps.renderer.glsurfaceview.GLSurfaceViewMapRenderer
+import com.mapbox.mapboxsdk.testapp.R
+import com.mapbox.mapboxsdk.testapp.activity.maplayout.SimpleMapActivity
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class GLSurfaceDestroyTest : BaseIntegrationTest() {
+
+  @get:Rule
+  var activityRule: ActivityTestRule<SimpleMapActivity> = ActivityTestRule(SimpleMapActivity::class.java)
+
+  private lateinit var mapView:MapView
+
+  @Test
+  @LargeTest
+  fun reopenSimpleMapActivity() {
+    device.waitForIdle()
+    mapView = activityRule.activity.findViewById(R.id.mapView)
+    device.pressHome()
+    device.waitForIdle()
+    (mapView.mapRenderer as GLSurfaceViewMapRenderer).onDrawFrame(null)
+    device.waitForIdle()
+  }
+}
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/MapboxApplication.java b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/MapboxApplication.java
index 71762c1e5f..5e9fb43f0d 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/MapboxApplication.java
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/MapboxApplication.java
@@ -54,9 +54,7 @@ public class MapboxApplication extends Application {
 
   private void initializeLogger() {
     Logger.setLoggerDefinition(new TimberLogger());
-    if (BuildConfig.DEBUG) {
-      Timber.plant(new DebugTree());
-    }
+    Timber.plant(new DebugTree());
   }
 
   private void initializeStrictMode() {