From e3c3159c2b27502ec3be5a98452edec22f9648e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Tue, 6 Nov 2018 12:57:14 +0100 Subject: [android] log Java exception and fail silently instead of throwing a native exception in the map state callbacks --- .../mapbox/mapboxsdk/maps/MapChangeReceiver.java | 137 +++++++++++++++------ .../java/com/mapbox/mapboxsdk/maps/MapView.java | 6 + .../mapboxsdk/maps/MapChangeReceiverTest.java | 122 ++++++++++++++++++ 3 files changed, 229 insertions(+), 36 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapChangeReceiver.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapChangeReceiver.java index 379f67aa7a..3eaa381239 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapChangeReceiver.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapChangeReceiver.java @@ -1,10 +1,15 @@ package com.mapbox.mapboxsdk.maps; +import com.mapbox.mapboxsdk.MapStrictMode; +import com.mapbox.mapboxsdk.log.Logger; + import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; class MapChangeReceiver implements NativeMapView.StateCallback { + private static final String TAG = "Mbgl-MapChangeReceiver"; + private final List onCameraWillChangeListenerList = new CopyOnWriteArrayList<>(); private final List onCameraIsChangingListenerList = new CopyOnWriteArrayList<>(); private final List onCameraDidChangeListenerList = new CopyOnWriteArrayList<>(); @@ -28,109 +33,169 @@ class MapChangeReceiver implements NativeMapView.StateCallback { @Override public void onCameraWillChange(boolean animated) { - if (!onCameraWillChangeListenerList.isEmpty()) { - for (MapView.OnCameraWillChangeListener onCameraWillChangeListener : onCameraWillChangeListenerList) { - onCameraWillChangeListener.onCameraWillChange(animated); + try { + if (!onCameraWillChangeListenerList.isEmpty()) { + for (MapView.OnCameraWillChangeListener onCameraWillChangeListener : onCameraWillChangeListenerList) { + onCameraWillChangeListener.onCameraWillChange(animated); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onCameraWillChange", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onCameraIsChanging() { - if (!onCameraIsChangingListenerList.isEmpty()) { - for (MapView.OnCameraIsChangingListener onCameraIsChangingListener : onCameraIsChangingListenerList) { - onCameraIsChangingListener.onCameraIsChanging(); + try { + if (!onCameraIsChangingListenerList.isEmpty()) { + for (MapView.OnCameraIsChangingListener onCameraIsChangingListener : onCameraIsChangingListenerList) { + onCameraIsChangingListener.onCameraIsChanging(); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onCameraIsChanging", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onCameraDidChange(boolean animated) { - if (!onCameraDidChangeListenerList.isEmpty()) { - for (MapView.OnCameraDidChangeListener onCameraDidChangeListener : onCameraDidChangeListenerList) { - onCameraDidChangeListener.onCameraDidChange(animated); + try { + if (!onCameraDidChangeListenerList.isEmpty()) { + for (MapView.OnCameraDidChangeListener onCameraDidChangeListener : onCameraDidChangeListenerList) { + onCameraDidChangeListener.onCameraDidChange(animated); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onCameraDidChange", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onWillStartLoadingMap() { - if (!onWillStartLoadingMapListenerList.isEmpty()) { - for (MapView.OnWillStartLoadingMapListener onWillStartLoadingMapListener : onWillStartLoadingMapListenerList) { - onWillStartLoadingMapListener.onWillStartLoadingMap(); + try { + if (!onWillStartLoadingMapListenerList.isEmpty()) { + for (MapView.OnWillStartLoadingMapListener onWillStartLoadingMapListener : onWillStartLoadingMapListenerList) { + onWillStartLoadingMapListener.onWillStartLoadingMap(); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onWillStartLoadingMap", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onDidFinishLoadingMap() { - if (!onDidFinishLoadingMapListenerList.isEmpty()) { - for (MapView.OnDidFinishLoadingMapListener onDidFinishLoadingMapListener : onDidFinishLoadingMapListenerList) { - onDidFinishLoadingMapListener.onDidFinishLoadingMap(); + try { + if (!onDidFinishLoadingMapListenerList.isEmpty()) { + for (MapView.OnDidFinishLoadingMapListener onDidFinishLoadingMapListener : onDidFinishLoadingMapListenerList) { + onDidFinishLoadingMapListener.onDidFinishLoadingMap(); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onDidFinishLoadingMap", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onDidFailLoadingMap(String error) { - if (!onDidFailLoadingMapListenerList.isEmpty()) { - for (MapView.OnDidFailLoadingMapListener onDidFailLoadingMapListener : onDidFailLoadingMapListenerList) { - onDidFailLoadingMapListener.onDidFailLoadingMap(error); + try { + if (!onDidFailLoadingMapListenerList.isEmpty()) { + for (MapView.OnDidFailLoadingMapListener onDidFailLoadingMapListener : onDidFailLoadingMapListenerList) { + onDidFailLoadingMapListener.onDidFailLoadingMap(error); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onDidFailLoadingMap", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onWillStartRenderingFrame() { - if (!onWillStartRenderingFrameList.isEmpty()) { - for (MapView.OnWillStartRenderingFrameListener listener : onWillStartRenderingFrameList) { - listener.onWillStartRenderingFrame(); + try { + if (!onWillStartRenderingFrameList.isEmpty()) { + for (MapView.OnWillStartRenderingFrameListener listener : onWillStartRenderingFrameList) { + listener.onWillStartRenderingFrame(); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onWillStartRenderingFrame", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onDidFinishRenderingFrame(boolean fully) { - if (!onDidFinishRenderingFrameList.isEmpty()) { - for (MapView.OnDidFinishRenderingFrameListener listener : onDidFinishRenderingFrameList) { - listener.onDidFinishRenderingFrame(fully); + try { + if (!onDidFinishRenderingFrameList.isEmpty()) { + for (MapView.OnDidFinishRenderingFrameListener listener : onDidFinishRenderingFrameList) { + listener.onDidFinishRenderingFrame(fully); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onDidFinishRenderingFrame", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onWillStartRenderingMap() { - if (!onWillStartRenderingMapListenerList.isEmpty()) { - for (MapView.OnWillStartRenderingMapListener listener : onWillStartRenderingMapListenerList) { - listener.onWillStartRenderingMap(); + try { + if (!onWillStartRenderingMapListenerList.isEmpty()) { + for (MapView.OnWillStartRenderingMapListener listener : onWillStartRenderingMapListenerList) { + listener.onWillStartRenderingMap(); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onWillStartRenderingMap", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onDidFinishRenderingMap(boolean fully) { - if (!onDidFinishRenderingMapListenerList.isEmpty()) { - for (MapView.OnDidFinishRenderingMapListener listener : onDidFinishRenderingMapListenerList) { - listener.onDidFinishRenderingMap(fully); + try { + if (!onDidFinishRenderingMapListenerList.isEmpty()) { + for (MapView.OnDidFinishRenderingMapListener listener : onDidFinishRenderingMapListenerList) { + listener.onDidFinishRenderingMap(fully); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onDidFinishRenderingMap", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onDidFinishLoadingStyle() { - if (!onDidFinishLoadingStyleListenerList.isEmpty()) { - for (MapView.OnDidFinishLoadingStyleListener listener : onDidFinishLoadingStyleListenerList) { - listener.onDidFinishLoadingStyle(); + try { + if (!onDidFinishLoadingStyleListenerList.isEmpty()) { + for (MapView.OnDidFinishLoadingStyleListener listener : onDidFinishLoadingStyleListenerList) { + listener.onDidFinishLoadingStyle(); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onDidFinishLoadingStyle", err); + MapStrictMode.strictModeViolation(err); } } @Override public void onSourceChanged(String sourceId) { - if (!onSourceChangedListenerList.isEmpty()) { - for (MapView.OnSourceChangedListener onSourceChangedListener : onSourceChangedListenerList) { - onSourceChangedListener.onSourceChangedListener(sourceId); + try { + if (!onSourceChangedListenerList.isEmpty()) { + for (MapView.OnSourceChangedListener onSourceChangedListener : onSourceChangedListenerList) { + onSourceChangedListener.onSourceChangedListener(sourceId); + } } + } catch (RuntimeException err) { + Logger.e(TAG, "Exception in onSourceChanged", err); + MapStrictMode.strictModeViolation(err); } } 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 7cb8245301..20bb49f140 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 @@ -789,6 +789,12 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback { mapChangeReceiver.removeOnSourceChangedListener(listener); } + /** + * Interface definition for a callback to be invoked when the camera will change. + *

+ * {@link MapView#addOnCameraWillChangeListener(OnCameraWillChangeListener)} + *

+ */ public interface OnCameraWillChangeListener { /** diff --git a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapChangeReceiverTest.java b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapChangeReceiverTest.java index 8954b24785..688b4badec 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapChangeReceiverTest.java +++ b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapChangeReceiverTest.java @@ -1,10 +1,16 @@ package com.mapbox.mapboxsdk.maps; +import com.mapbox.mapboxsdk.log.Logger; +import com.mapbox.mapboxsdk.log.LoggerDefinition; + import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; /** @@ -52,6 +58,9 @@ public class MapChangeReceiverTest { @Mock private MapView.OnSourceChangedListener onSourceChangedListener; + @Mock + private LoggerDefinition loggerDefinition; + @Before public void beforeTest() { MockitoAnnotations.initMocks(this); @@ -63,9 +72,17 @@ public class MapChangeReceiverTest { mapChangeEventManager.addOnCameraWillChangeListener(onCameraWillChangeListener); mapChangeEventManager.onCameraWillChange(false); verify(onCameraWillChangeListener).onCameraWillChange(false); + mapChangeEventManager.removeOnCameraWillChangeListener(onCameraWillChangeListener); mapChangeEventManager.onCameraWillChange(false); verify(onCameraWillChangeListener).onCameraWillChange(false); + + mapChangeEventManager.addOnCameraWillChangeListener(onCameraWillChangeListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onCameraWillChangeListener).onCameraWillChange(false); + mapChangeEventManager.onCameraWillChange(false); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -76,6 +93,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnCameraWillChangeListener(onCameraWillChangeListener); mapChangeEventManager.onCameraWillChange(true); verify(onCameraWillChangeListener).onCameraWillChange(true); + + mapChangeEventManager.addOnCameraWillChangeListener(onCameraWillChangeListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onCameraWillChangeListener).onCameraWillChange(true); + mapChangeEventManager.onCameraWillChange(true); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -86,6 +110,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnCameraIsChangingListener(onCameraIsChangingListener); mapChangeEventManager.onCameraIsChanging(); verify(onCameraIsChangingListener).onCameraIsChanging(); + + mapChangeEventManager.addOnCameraIsChangingListener(onCameraIsChangingListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onCameraIsChangingListener).onCameraIsChanging(); + mapChangeEventManager.onCameraIsChanging(); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -96,6 +127,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnCameraDidChangeListener(onCameraDidChangeListener); mapChangeEventManager.onCameraDidChange(false); verify(onCameraDidChangeListener).onCameraDidChange(false); + + mapChangeEventManager.addOnCameraDidChangeListener(onCameraDidChangeListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onCameraDidChangeListener).onCameraDidChange(false); + mapChangeEventManager.onCameraDidChange(false); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -106,6 +144,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnCameraDidChangeListener(onCameraDidChangeListener); mapChangeEventManager.onCameraDidChange(true); verify(onCameraDidChangeListener).onCameraDidChange(true); + + mapChangeEventManager.addOnCameraDidChangeListener(onCameraDidChangeListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onCameraDidChangeListener).onCameraDidChange(true); + mapChangeEventManager.onCameraDidChange(true); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -116,6 +161,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnWillStartLoadingMapListener(onWillStartLoadingMapListener); mapChangeEventManager.onWillStartLoadingMap(); verify(onWillStartLoadingMapListener).onWillStartLoadingMap(); + + mapChangeEventManager.addOnWillStartLoadingMapListener(onWillStartLoadingMapListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onWillStartLoadingMapListener).onWillStartLoadingMap(); + mapChangeEventManager.onWillStartLoadingMap(); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -126,6 +178,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFinishLoadingMapListener(onDidFinishLoadingMapListener); mapChangeEventManager.onDidFinishLoadingMap(); verify(onDidFinishLoadingMapListener).onDidFinishLoadingMap(); + + mapChangeEventManager.addOnDidFinishLoadingMapListener(onDidFinishLoadingMapListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFinishLoadingMapListener).onDidFinishLoadingMap(); + mapChangeEventManager.onDidFinishLoadingMap(); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -136,6 +195,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFailLoadingMapListener(onDidFailLoadingMapListener); mapChangeEventManager.onDidFailLoadingMap(TEST_STRING); verify(onDidFailLoadingMapListener).onDidFailLoadingMap(TEST_STRING); + + mapChangeEventManager.addOnDidFailLoadingMapListener(onDidFailLoadingMapListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFailLoadingMapListener).onDidFailLoadingMap(TEST_STRING); + mapChangeEventManager.onDidFailLoadingMap(TEST_STRING); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -146,6 +212,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnWillStartRenderingFrameListener(onWillStartRenderingFrameListener); mapChangeEventManager.onWillStartRenderingFrame(); verify(onWillStartRenderingFrameListener).onWillStartRenderingFrame(); + + mapChangeEventManager.addOnWillStartRenderingFrameListener(onWillStartRenderingFrameListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onWillStartRenderingFrameListener).onWillStartRenderingFrame(); + mapChangeEventManager.onWillStartRenderingFrame(); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -156,6 +229,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFinishRenderingFrameListener(onDidFinishRenderingFrameListener); mapChangeEventManager.onDidFinishRenderingFrame(true); verify(onDidFinishRenderingFrameListener).onDidFinishRenderingFrame(true); + + mapChangeEventManager.addOnDidFinishRenderingFrameListener(onDidFinishRenderingFrameListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFinishRenderingFrameListener).onDidFinishRenderingFrame(true); + mapChangeEventManager.onDidFinishRenderingFrame(true); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -166,6 +246,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFinishRenderingFrameListener(onDidFinishRenderingFrameListener); mapChangeEventManager.onDidFinishRenderingFrame(false); verify(onDidFinishRenderingFrameListener).onDidFinishRenderingFrame(false); + + mapChangeEventManager.addOnDidFinishRenderingFrameListener(onDidFinishRenderingFrameListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFinishRenderingFrameListener).onDidFinishRenderingFrame(false); + mapChangeEventManager.onDidFinishRenderingFrame(false); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -176,6 +263,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnWillStartRenderingMapListener(onWillStartRenderingMapListener); mapChangeEventManager.onWillStartRenderingMap(); verify(onWillStartRenderingMapListener).onWillStartRenderingMap(); + + mapChangeEventManager.addOnWillStartRenderingMapListener(onWillStartRenderingMapListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onWillStartRenderingMapListener).onWillStartRenderingMap(); + mapChangeEventManager.onWillStartRenderingMap(); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -186,6 +280,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFinishRenderingMapListener(onDidFinishRenderingMapListener); mapChangeEventManager.onDidFinishRenderingMap(true); verify(onDidFinishRenderingMapListener).onDidFinishRenderingMap(true); + + mapChangeEventManager.addOnDidFinishRenderingMapListener(onDidFinishRenderingMapListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFinishRenderingMapListener).onDidFinishRenderingMap(true); + mapChangeEventManager.onDidFinishRenderingMap(true); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -196,6 +297,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFinishRenderingMapListener(onDidFinishRenderingMapListener); mapChangeEventManager.onDidFinishRenderingMap(false); verify(onDidFinishRenderingMapListener).onDidFinishRenderingMap(false); + + mapChangeEventManager.addOnDidFinishRenderingMapListener(onDidFinishRenderingMapListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFinishRenderingMapListener).onDidFinishRenderingMap(false); + mapChangeEventManager.onDidFinishRenderingMap(false); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -206,6 +314,13 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnDidFinishLoadingStyleListener(onDidFinishLoadingStyleListener); mapChangeEventManager.onDidFinishLoadingStyle(); verify(onDidFinishLoadingStyleListener).onDidFinishLoadingStyle(); + + mapChangeEventManager.addOnDidFinishLoadingStyleListener(onDidFinishLoadingStyleListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onDidFinishLoadingStyleListener).onDidFinishLoadingStyle(); + mapChangeEventManager.onDidFinishLoadingStyle(); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } @Test @@ -216,5 +331,12 @@ public class MapChangeReceiverTest { mapChangeEventManager.removeOnSourceChangedListener(onSourceChangedListener); mapChangeEventManager.onSourceChanged(TEST_STRING); verify(onSourceChangedListener).onSourceChangedListener(TEST_STRING); + + mapChangeEventManager.addOnSourceChangedListener(onSourceChangedListener); + Logger.setLoggerDefinition(loggerDefinition); + Exception err = new RuntimeException(); + doThrow(err).when(onSourceChangedListener).onSourceChangedListener(TEST_STRING); + mapChangeEventManager.onSourceChanged(TEST_STRING); + verify(loggerDefinition).e(anyString(), anyString(), eq(err)); } } -- cgit v1.2.1