From f2ebfdbfd570ae7bd95c613116c17711732c1e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Tue, 15 Jan 2019 18:10:28 +0100 Subject: [android] do not invoke #onCancel when animation is scheduled from #onFinish block --- .../java/com/mapbox/mapboxsdk/maps/Transform.java | 16 ++-- .../com/mapbox/mapboxsdk/maps/MapboxMapTest.kt | 37 +++++----- .../com/mapbox/mapboxsdk/maps/TransformTest.kt | 86 ++++++++++++++++++++++ 3 files changed, 115 insertions(+), 24 deletions(-) create mode 100644 platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/TransformTest.kt diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Transform.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Transform.java index 9356b112ba..c40994d7ca 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Transform.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Transform.java @@ -5,6 +5,7 @@ import android.os.Handler; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.UiThread; + import com.mapbox.mapboxsdk.camera.CameraPosition; import com.mapbox.mapboxsdk.camera.CameraUpdate; import com.mapbox.mapboxsdk.camera.CameraUpdateFactory; @@ -77,13 +78,15 @@ final class Transform implements MapView.OnCameraDidChangeListener { if (animated) { invalidateCameraPosition(); if (cameraCancelableCallback != null) { + final MapboxMap.CancelableCallback callback = cameraCancelableCallback; + + // nullification has to happen before Handler#post, see https://github.com/robolectric/robolectric/issues/1306 + cameraCancelableCallback = null; + handler.post(new Runnable() { @Override public void run() { - if (cameraCancelableCallback != null) { - cameraCancelableCallback.onFinish(); - cameraCancelableCallback = null; - } + callback.onFinish(); } }); } @@ -173,13 +176,16 @@ final class Transform implements MapView.OnCameraDidChangeListener { if (cameraCancelableCallback != null) { final MapboxMap.CancelableCallback callback = cameraCancelableCallback; cameraChangeDispatcher.onCameraIdle(); + + // nullification has to happen before Handler#post, see https://github.com/robolectric/robolectric/issues/1306 + cameraCancelableCallback = null; + handler.post(new Runnable() { @Override public void run() { callback.onCancel(); } }); - cameraCancelableCallback = null; } // cancel ongoing transitions diff --git a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapboxMapTest.kt b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapboxMapTest.kt index 4c67049b39..1c0d1f76b7 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapboxMapTest.kt +++ b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/MapboxMapTest.kt @@ -22,23 +22,23 @@ class MapboxMapTest { private lateinit var nativeMapView: NativeMapView + private lateinit var transform: Transform + @Before fun setup() { val cameraChangeDispatcher = spyk() - val mapView = mockk() nativeMapView = mockk() - mapboxMap = MapboxMap(nativeMapView, Transform(mapView, nativeMapView, cameraChangeDispatcher), null, null, null, cameraChangeDispatcher) + transform = mockk() + mapboxMap = MapboxMap(nativeMapView, transform, null, null, null, cameraChangeDispatcher) every { nativeMapView.styleUrl = any() } answers {} every { nativeMapView.transitionOptions = any() } answers {} every { nativeMapView.isDestroyed } returns false - every { nativeMapView.cameraPosition } returns CameraPosition.DEFAULT - every { nativeMapView.cancelTransitions() } answers {} - every { nativeMapView.jumpTo(any(), any(), any(), any()) } answers {} - every { nativeMapView.minZoom = any() } answers {} - every { nativeMapView.maxZoom = any() } answers {} every { nativeMapView.setOnFpsChangedListener(any()) } answers {} every { nativeMapView.prefetchTiles = any() } answers {} every { nativeMapView.setLatLngBounds(any()) } answers {} + every { transform.minZoom = any() } answers {} + every { transform.maxZoom = any() } answers {} + every { transform.moveCamera(any(), any(), any()) } answers {} mapboxMap.injectLocationComponent(spyk()) mapboxMap.setStyle(Style.MAPBOX_STREETS) mapboxMap.onFinishLoadingStyle() @@ -51,27 +51,26 @@ class MapboxMapTest { verify { nativeMapView.transitionOptions = expected } } - @Test - fun testMoveCamera() { - val callback = mockk() - every { callback.onFinish() } answers {} - val target = LatLng(1.0, 2.0) - val expected = CameraPosition.Builder().target(target).build() - mapboxMap.moveCamera(CameraUpdateFactory.newCameraPosition(expected), callback) - verify { nativeMapView.jumpTo(target, -1.0, -1.0, -1.0) } - verify { callback.onFinish() } - } + @Test + fun testMoveCamera() { + val callback = mockk() + val target = LatLng(1.0, 2.0) + val expected = CameraPosition.Builder().target(target).build() + val update = CameraUpdateFactory.newCameraPosition(expected) + mapboxMap.moveCamera(update, callback) + verify { transform.moveCamera(mapboxMap, update, callback) } + } @Test fun testMinZoom() { mapboxMap.setMinZoomPreference(10.0) - verify { nativeMapView.minZoom = 10.0 } + verify { transform.minZoom = 10.0 } } @Test fun testMaxZoom() { mapboxMap.setMaxZoomPreference(10.0) - verify { nativeMapView.maxZoom = 10.0 } + verify { transform.maxZoom = 10.0 } } @Test diff --git a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/TransformTest.kt b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/TransformTest.kt new file mode 100644 index 0000000000..8c985ca3a8 --- /dev/null +++ b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/TransformTest.kt @@ -0,0 +1,86 @@ +package com.mapbox.mapboxsdk.maps + +import com.mapbox.mapboxsdk.camera.CameraPosition +import com.mapbox.mapboxsdk.camera.CameraUpdateFactory +import com.mapbox.mapboxsdk.geometry.LatLng +import io.mockk.* +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class TransformTest { + + private lateinit var mapView: MapView + private lateinit var nativeMapView: NativeMapView + private lateinit var transform: Transform + + @Before + fun setup() { + val cameraChangeDispatcher = spyk() + mapView = mockk() + nativeMapView = mockk() + transform = Transform(mapView, nativeMapView, cameraChangeDispatcher) + every { nativeMapView.isDestroyed } returns false + every { nativeMapView.cameraPosition } returns CameraPosition.DEFAULT + every { nativeMapView.cancelTransitions() } answers {} + every { nativeMapView.jumpTo(any(), any(), any(), any()) } answers {} + every { nativeMapView.flyTo(any(), any(), any(), any(), any()) } answers {} + every { nativeMapView.minZoom = any() } answers {} + every { nativeMapView.maxZoom = any() } answers {} + } + + @Test + fun testMoveCamera() { + val mapboxMap = mockk() + every { mapboxMap.cameraPosition } answers { CameraPosition.DEFAULT } + + val callback = mockk() + every { callback.onFinish() } answers {} + + val target = LatLng(1.0, 2.0) + val expected = CameraPosition.Builder().target(target).build() + val update = CameraUpdateFactory.newCameraPosition(expected) + transform.moveCamera(mapboxMap, update, callback) + + verify { nativeMapView.jumpTo(target, -1.0, -1.0, -1.0) } + verify { callback.onFinish() } + } + + @Test + fun testMinZoom() { + transform.minZoom = 10.0 + verify { nativeMapView.minZoom = 10.0 } + } + + @Test + fun testMaxZoom() { + transform.maxZoom = 10.0 + verify { nativeMapView.maxZoom = 10.0 } + } + + @Test + fun testCancelNotInvokedFromOnFinish() { + val slot = slot() + every { mapView.addOnCameraDidChangeListener(capture(slot)) } answers { slot.captured.onCameraDidChange(true) } + every { mapView.removeOnCameraDidChangeListener(any()) } answers {} + // regression test for https://github.com/mapbox/mapbox-gl-native/issues/13735 + val mapboxMap = mockk() + every { mapboxMap.cameraPosition } answers { CameraPosition.DEFAULT } + + val target = LatLng(1.0, 2.0) + val expected = CameraPosition.Builder().target(target).build() + + val callback = object : MapboxMap.CancelableCallback { + override fun onCancel() { + throw IllegalStateException("onCancel shouldn't be called from onFinish") + } + + override fun onFinish() { + transform.animateCamera(mapboxMap, CameraUpdateFactory.newCameraPosition(expected), 500, null) + } + } + transform.animateCamera(mapboxMap, CameraUpdateFactory.newCameraPosition(expected), 500, callback) + } +} \ No newline at end of file -- cgit v1.2.1